From c17cf2b2f7a79e12559acbaa5310a7a13607439d Mon Sep 17 00:00:00 2001 From: Tymoteusz Jankowski Date: Sun, 1 Dec 2019 13:23:09 +0100 Subject: [PATCH 01/14] dvc/dagascii: Use less instead of AsciiCanvas._do_draw Uses less CLI tool to paging the output in the interactive mode while doing e.g. `dvc pipeline show ..`. Fixes #2807 --- dvc/dagascii.py | 141 +++++++++++++++++------------------------------- dvc/system.py | 35 ------------ setup.py | 1 - 3 files changed, 48 insertions(+), 129 deletions(-) diff --git a/dvc/dagascii.py b/dvc/dagascii.py index 6f324b865b..e7058c3fd5 100644 --- a/dvc/dagascii.py +++ b/dvc/dagascii.py @@ -2,8 +2,12 @@ from __future__ import print_function from __future__ import unicode_literals +import logging import math +import os +import pydoc import sys +from distutils.spawn import find_executable from grandalf.graphs import Edge from grandalf.graphs import Graph @@ -12,6 +16,44 @@ from grandalf.routing import EdgeViewer from grandalf.routing import route_with_lines +from dvc.utils import boxify + + +logger = logging.getLogger(__name__) + + +DEFAULT_PAGER = "less" +DEFAULT_PAGER_FORMATTED = "{} --chop-long-lines --clear-screen".format( + DEFAULT_PAGER +) +DVC_PAGER_VAR_NAME = "DVC_PAGER" +DVC_PAGER_INFO = """\ +Less command line tool is missing. +Install & add less tool to your PATH env. var. to automatically send output \ +to pager. +Also, you can override default pager via {} env. var. +""".format( + DVC_PAGER_VAR_NAME +) + + +def find_pager(pager_cmd, pager_cmd_with_format, pager_env_name): + if sys.stdout.isatty(): + less_found = find_executable(pager_cmd) is not None + if less_found: + pager_cmd = os.getenv(pager_env_name, pager_cmd_with_format) + + def less_pager(text): + return pydoc.tempfilepager(pydoc.plain(text), pager_cmd) + + pager = less_pager + else: + logger.info(boxify(DVC_PAGER_INFO, border_color="yellow")) + pager = pydoc.plainpager + else: + pager = pydoc.plainpager + return pager + class VertexViewer(object): """Class to define vertex box boundaries that will be accounted for during @@ -60,99 +102,12 @@ def __init__(self, cols, lines): def draw(self): """Draws ASCII canvas on the screen.""" - if sys.stdout.isatty(): # pragma: no cover - from asciimatics.screen import Screen - - Screen.wrapper(self._do_draw) - else: - for line in self.canvas: - print("".join(line)) - - def _do_draw(self, screen): # pragma: no cover - # pylint: disable=too-many-locals - # pylint: disable=too-many-branches, too-many-statements - from dvc.system import System - from asciimatics.event import KeyboardEvent - - offset_x = 0 - offset_y = 0 - smaxrow, smaxcol = screen.dimensions - assert smaxrow > 1 - assert smaxcol > 1 - smaxrow -= 1 - smaxcol -= 1 - - if self.lines + 1 > smaxrow: - max_y = self.lines + 1 - smaxrow - else: - max_y = 0 - - if self.cols + 1 > smaxcol: - max_x = self.cols + 1 - smaxcol - else: - max_x = 0 - - while True: - for y in range(smaxrow + 1): - y_index = offset_y + y - line = [] - for x in range(smaxcol + 1): - x_index = offset_x + x - if ( - len(self.canvas) > y_index - and len(self.canvas[y_index]) > x_index - ): - line.append(self.canvas[y_index][x_index]) - else: - line.append(" ") - assert len(line) == (smaxcol + 1) - screen.print_at("".join(line), 0, y) - - screen.refresh() - - # NOTE: get_event() doesn't block by itself, - # so we have to do the blocking ourselves. - # - # NOTE: using this workaround while waiting for PR [1] - # to get merged and released. After that need to adjust - # asciimatics version requirements. - # - # [1] https://github.com/peterbrittain/asciimatics/pull/188 - System.wait_for_input(self.TIMEOUT) - - event = screen.get_event() - if not isinstance(event, KeyboardEvent): - continue - - k = event.key_code - if k == screen.KEY_DOWN or k == ord("s"): - offset_y += 1 - elif k == screen.KEY_PAGE_DOWN or k == ord("S"): - offset_y += smaxrow - elif k == screen.KEY_UP or k == ord("w"): - offset_y -= 1 - elif k == screen.KEY_PAGE_UP or k == ord("W"): - offset_y -= smaxrow - elif k == screen.KEY_RIGHT or k == ord("d"): - offset_x += 1 - elif k == ord("D"): - offset_x += smaxcol - elif k == screen.KEY_LEFT or k == ord("a"): - offset_x -= 1 - elif k == ord("A"): - offset_x -= smaxcol - elif k == ord("q") or k == ord("Q"): - break - - if offset_y > max_y: - offset_y = max_y - elif offset_y < 0: - offset_y = 0 - - if offset_x > max_x: - offset_x = max_x - elif offset_x < 0: - offset_x = 0 + pager = find_pager( + DEFAULT_PAGER, DEFAULT_PAGER_FORMATTED, DVC_PAGER_VAR_NAME + ) + lines = map("".join, self.canvas) + joined_lines = os.linesep.join(lines) + pager(joined_lines) def point(self, x, y, char): """Create a point on ASCII canvas. diff --git a/dvc/system.py b/dvc/system.py index 1a8dbef70b..2195008337 100644 --- a/dvc/system.py +++ b/dvc/system.py @@ -218,41 +218,6 @@ def inode(path): assert inode < 2 ** 64 return inode - @staticmethod - def _wait_for_input_windows(timeout): - import sys - import ctypes - import msvcrt - from ctypes.wintypes import DWORD, HANDLE - - # https://docs.microsoft.com/en-us/windows/desktop/api/synchapi/nf-synchapi-waitforsingleobject - from win32event import WAIT_OBJECT_0, WAIT_TIMEOUT - - func = ctypes.windll.kernel32.WaitForSingleObject - func.argtypes = [HANDLE, DWORD] - func.restype = DWORD - - rc = func(msvcrt.get_osfhandle(sys.stdin.fileno()), timeout * 1000) - if rc not in [WAIT_OBJECT_0, WAIT_TIMEOUT]: - raise RuntimeError(rc) - - @staticmethod - def _wait_for_input_posix(timeout): - import sys - import select - - try: - select.select([sys.stdin], [], [], timeout) - except select.error: - pass - - @staticmethod - def wait_for_input(timeout): - if System.is_unix(): - return System._wait_for_input_posix(timeout) - else: - return System._wait_for_input_windows(timeout) - @staticmethod def is_symlink(path): path = fspath(path) diff --git a/setup.py b/setup.py index b9b16f014d..c3903c6be9 100644 --- a/setup.py +++ b/setup.py @@ -66,7 +66,6 @@ def run(self): "jsonpath-ng>=1.4.3", "requests>=2.22.0", "grandalf==0.6", - "asciimatics>=1.10.0", "distro>=1.3.0", "appdirs>=1.4.3", "treelib>=1.5.5", From cade3b396c4c702b4003d9a0a6a80c7db4a46801 Mon Sep 17 00:00:00 2001 From: Tymoteusz Jankowski Date: Sun, 1 Dec 2019 19:03:51 +0100 Subject: [PATCH 02/14] Add is_exec_found to utils and use it in dagascii module --- dvc/dagascii.py | 6 ++---- dvc/utils/__init__.py | 5 +++++ tests/func/test_utils.py | 10 ++++++++++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/dvc/dagascii.py b/dvc/dagascii.py index e7058c3fd5..68459120c4 100644 --- a/dvc/dagascii.py +++ b/dvc/dagascii.py @@ -7,7 +7,6 @@ import os import pydoc import sys -from distutils.spawn import find_executable from grandalf.graphs import Edge from grandalf.graphs import Graph @@ -16,7 +15,7 @@ from grandalf.routing import EdgeViewer from grandalf.routing import route_with_lines -from dvc.utils import boxify +from dvc.utils import boxify, is_exec_found logger = logging.getLogger(__name__) @@ -39,8 +38,7 @@ def find_pager(pager_cmd, pager_cmd_with_format, pager_env_name): if sys.stdout.isatty(): - less_found = find_executable(pager_cmd) is not None - if less_found: + if is_exec_found(pager_cmd): pager_cmd = os.getenv(pager_env_name, pager_cmd_with_format) def less_pager(text): diff --git a/dvc/utils/__init__.py b/dvc/utils/__init__.py index 68ba418778..f1053eb9c7 100644 --- a/dvc/utils/__init__.py +++ b/dvc/utils/__init__.py @@ -420,3 +420,8 @@ def resolve_output(inp, out): if os.path.isdir(out): return os.path.join(out, name) return out + + +def is_exec_found(exec_name): + cmd = '({}) 2>/dev/null'.format(exec_name) + return hasattr(os, 'system') and os.system(cmd) == 0 diff --git a/tests/func/test_utils.py b/tests/func/test_utils.py index 594fdc332d..40c575e4f4 100644 --- a/tests/func/test_utils.py +++ b/tests/func/test_utils.py @@ -88,3 +88,13 @@ def test_makedirs_permissions(tmpdir): assert stat.S_IMODE(os.stat(test_dir).st_mode) == dir_mode assert stat.S_IMODE(os.stat(intermediate_dir).st_mode) == dir_mode + + +def test_is_exec_found_returns_true_when_program_exists(): + result = utils.is_exec_found("python") + assert result == True + + +def test_is_exec_found_returns_false_when_program_is_missing(): + result = utils.is_exec_found("some-missing-program") + assert result == False From 3e163f19343719bb4d13d9f67d6d45df1447ab22 Mon Sep 17 00:00:00 2001 From: Tymoteusz Jankowski Date: Sun, 1 Dec 2019 19:13:56 +0100 Subject: [PATCH 03/14] Rename and move DVC_PAGER_ENV_NAME to env.py file --- dvc/dagascii.py | 14 ++++++-------- dvc/env.py | 1 + 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/dvc/dagascii.py b/dvc/dagascii.py index 68459120c4..87a0176bff 100644 --- a/dvc/dagascii.py +++ b/dvc/dagascii.py @@ -15,6 +15,7 @@ from grandalf.routing import EdgeViewer from grandalf.routing import route_with_lines +from dvc.env import DVC_PAGER from dvc.utils import boxify, is_exec_found @@ -25,21 +26,20 @@ DEFAULT_PAGER_FORMATTED = "{} --chop-long-lines --clear-screen".format( DEFAULT_PAGER ) -DVC_PAGER_VAR_NAME = "DVC_PAGER" DVC_PAGER_INFO = """\ Less command line tool is missing. Install & add less tool to your PATH env. var. to automatically send output \ to pager. Also, you can override default pager via {} env. var. """.format( - DVC_PAGER_VAR_NAME + DVC_PAGER ) -def find_pager(pager_cmd, pager_cmd_with_format, pager_env_name): +def find_pager(): if sys.stdout.isatty(): - if is_exec_found(pager_cmd): - pager_cmd = os.getenv(pager_env_name, pager_cmd_with_format) + if is_exec_found(DEFAULT_PAGER): + pager_cmd = os.getenv(DVC_PAGER, DEFAULT_PAGER_FORMATTED) def less_pager(text): return pydoc.tempfilepager(pydoc.plain(text), pager_cmd) @@ -100,9 +100,7 @@ def __init__(self, cols, lines): def draw(self): """Draws ASCII canvas on the screen.""" - pager = find_pager( - DEFAULT_PAGER, DEFAULT_PAGER_FORMATTED, DVC_PAGER_VAR_NAME - ) + pager = find_pager() lines = map("".join, self.canvas) joined_lines = os.linesep.join(lines) pager(joined_lines) diff --git a/dvc/env.py b/dvc/env.py index be49679127..9b5ea67a80 100644 --- a/dvc/env.py +++ b/dvc/env.py @@ -1 +1,2 @@ DVC_DAEMON = "DVC_DAEMON" +DVC_PAGER = "DVC_PAGER" From a6a3cbcffa2c98bf929880452d25e2055d4cf9a5 Mon Sep 17 00:00:00 2001 From: Tymoteusz Jankowski Date: Sun, 1 Dec 2019 19:17:28 +0100 Subject: [PATCH 04/14] Reduce nested ifs in find_pager --- dvc/dagascii.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/dvc/dagascii.py b/dvc/dagascii.py index 87a0176bff..bf23d88c09 100644 --- a/dvc/dagascii.py +++ b/dvc/dagascii.py @@ -37,20 +37,19 @@ def find_pager(): - if sys.stdout.isatty(): - if is_exec_found(DEFAULT_PAGER): - pager_cmd = os.getenv(DVC_PAGER, DEFAULT_PAGER_FORMATTED) + if not sys.stdout.isatty(): + return pydoc.plainpager - def less_pager(text): - return pydoc.tempfilepager(pydoc.plain(text), pager_cmd) + if is_exec_found(DEFAULT_PAGER): + pager_cmd = os.getenv(DVC_PAGER, DEFAULT_PAGER_FORMATTED) - pager = less_pager - else: - logger.info(boxify(DVC_PAGER_INFO, border_color="yellow")) - pager = pydoc.plainpager - else: - pager = pydoc.plainpager - return pager + def less_pager(text): + return pydoc.tempfilepager(pydoc.plain(text), pager_cmd) + + return less_pager + + logger.info(boxify(DVC_PAGER_INFO, border_color="yellow")) + return pydoc.plainpager class VertexViewer(object): From f79711d15502f64c9ff4ff5b4cc1b30ba38e8fa4 Mon Sep 17 00:00:00 2001 From: Tymoteusz Jankowski Date: Sun, 1 Dec 2019 19:48:56 +0100 Subject: [PATCH 05/14] Fix code checks --- dvc/utils/__init__.py | 4 ++-- tests/func/test_utils.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dvc/utils/__init__.py b/dvc/utils/__init__.py index f1053eb9c7..4a186bfea9 100644 --- a/dvc/utils/__init__.py +++ b/dvc/utils/__init__.py @@ -423,5 +423,5 @@ def resolve_output(inp, out): def is_exec_found(exec_name): - cmd = '({}) 2>/dev/null'.format(exec_name) - return hasattr(os, 'system') and os.system(cmd) == 0 + cmd = "({}) 2>/dev/null".format(exec_name) + return hasattr(os, "system") and os.system(cmd) == 0 diff --git a/tests/func/test_utils.py b/tests/func/test_utils.py index 40c575e4f4..3b75573a40 100644 --- a/tests/func/test_utils.py +++ b/tests/func/test_utils.py @@ -92,9 +92,9 @@ def test_makedirs_permissions(tmpdir): def test_is_exec_found_returns_true_when_program_exists(): result = utils.is_exec_found("python") - assert result == True + assert result is True def test_is_exec_found_returns_false_when_program_is_missing(): result = utils.is_exec_found("some-missing-program") - assert result == False + assert result is False From ab3ccf0243e6d957762fd2ebb0d615e3891a993a Mon Sep 17 00:00:00 2001 From: Tymoteusz Jankowski Date: Mon, 2 Dec 2019 20:27:20 +0100 Subject: [PATCH 06/14] Use os.devnull + do not check if system exists for os module --- dvc/utils/__init__.py | 4 ++-- tests/func/test_utils.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dvc/utils/__init__.py b/dvc/utils/__init__.py index 4a186bfea9..43de56dea2 100644 --- a/dvc/utils/__init__.py +++ b/dvc/utils/__init__.py @@ -423,5 +423,5 @@ def resolve_output(inp, out): def is_exec_found(exec_name): - cmd = "({}) 2>/dev/null".format(exec_name) - return hasattr(os, "system") and os.system(cmd) == 0 + cmd = "({}) 2>{}".format(exec_name, os.devnull) + return os.system(cmd) == 0 diff --git a/tests/func/test_utils.py b/tests/func/test_utils.py index 3b75573a40..33fad8a828 100644 --- a/tests/func/test_utils.py +++ b/tests/func/test_utils.py @@ -91,7 +91,7 @@ def test_makedirs_permissions(tmpdir): def test_is_exec_found_returns_true_when_program_exists(): - result = utils.is_exec_found("python") + result = utils.is_exec_found("echo") assert result is True From 897795837194a44a3b828d2e56969c86122d9ac7 Mon Sep 17 00:00:00 2001 From: Tymoteusz Jankowski Date: Mon, 2 Dec 2019 20:36:51 +0100 Subject: [PATCH 07/14] Make pager info box to be a simple warning --- dvc/dagascii.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/dvc/dagascii.py b/dvc/dagascii.py index bf23d88c09..bbf0cca5fc 100644 --- a/dvc/dagascii.py +++ b/dvc/dagascii.py @@ -16,7 +16,7 @@ from grandalf.routing import route_with_lines from dvc.env import DVC_PAGER -from dvc.utils import boxify, is_exec_found +from dvc.utils import is_exec_found logger = logging.getLogger(__name__) @@ -26,14 +26,6 @@ DEFAULT_PAGER_FORMATTED = "{} --chop-long-lines --clear-screen".format( DEFAULT_PAGER ) -DVC_PAGER_INFO = """\ -Less command line tool is missing. -Install & add less tool to your PATH env. var. to automatically send output \ -to pager. -Also, you can override default pager via {} env. var. -""".format( - DVC_PAGER -) def find_pager(): @@ -48,7 +40,10 @@ def less_pager(text): return less_pager - logger.info(boxify(DVC_PAGER_INFO, border_color="yellow")) + logger.warning( + "Unable to find `less` in the PATH. Check out " + "man.dvc.org/doc/command-reference/pipeline/show for more info." + ) return pydoc.plainpager From c864ca938437736638c1b4582b3e162fe44ba899 Mon Sep 17 00:00:00 2001 From: Tymoteusz Jankowski Date: Wed, 4 Dec 2019 20:10:46 +0100 Subject: [PATCH 08/14] Move is_exec_found(..) to dvc/dagascii.py --- dvc/dagascii.py | 3 +-- dvc/utils/__init__.py | 5 ----- tests/func/test_utils.py | 10 ---------- 3 files changed, 1 insertion(+), 17 deletions(-) diff --git a/dvc/dagascii.py b/dvc/dagascii.py index bbf0cca5fc..240bdc37b7 100644 --- a/dvc/dagascii.py +++ b/dvc/dagascii.py @@ -16,7 +16,6 @@ from grandalf.routing import route_with_lines from dvc.env import DVC_PAGER -from dvc.utils import is_exec_found logger = logging.getLogger(__name__) @@ -32,7 +31,7 @@ def find_pager(): if not sys.stdout.isatty(): return pydoc.plainpager - if is_exec_found(DEFAULT_PAGER): + if os.system("({}) 2>{}".format(DEFAULT_PAGER, os.devnull)) == 0: pager_cmd = os.getenv(DVC_PAGER, DEFAULT_PAGER_FORMATTED) def less_pager(text): diff --git a/dvc/utils/__init__.py b/dvc/utils/__init__.py index 43de56dea2..68ba418778 100644 --- a/dvc/utils/__init__.py +++ b/dvc/utils/__init__.py @@ -420,8 +420,3 @@ def resolve_output(inp, out): if os.path.isdir(out): return os.path.join(out, name) return out - - -def is_exec_found(exec_name): - cmd = "({}) 2>{}".format(exec_name, os.devnull) - return os.system(cmd) == 0 diff --git a/tests/func/test_utils.py b/tests/func/test_utils.py index 33fad8a828..594fdc332d 100644 --- a/tests/func/test_utils.py +++ b/tests/func/test_utils.py @@ -88,13 +88,3 @@ def test_makedirs_permissions(tmpdir): assert stat.S_IMODE(os.stat(test_dir).st_mode) == dir_mode assert stat.S_IMODE(os.stat(intermediate_dir).st_mode) == dir_mode - - -def test_is_exec_found_returns_true_when_program_exists(): - result = utils.is_exec_found("echo") - assert result is True - - -def test_is_exec_found_returns_false_when_program_is_missing(): - result = utils.is_exec_found("some-missing-program") - assert result is False From 128f60ef68227c8f7bcfe2de79de18eab215121f Mon Sep 17 00:00:00 2001 From: Tymoteusz Jankowski Date: Wed, 4 Dec 2019 20:49:24 +0100 Subject: [PATCH 09/14] Add test_dagascii.py + make find_pager() use DVC_PAGER --- dvc/dagascii.py | 19 ++++++++++++++----- tests/unit/test_dagascii.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 5 deletions(-) create mode 100644 tests/unit/test_dagascii.py diff --git a/dvc/dagascii.py b/dvc/dagascii.py index 240bdc37b7..a686392f8e 100644 --- a/dvc/dagascii.py +++ b/dvc/dagascii.py @@ -31,13 +31,22 @@ def find_pager(): if not sys.stdout.isatty(): return pydoc.plainpager - if os.system("({}) 2>{}".format(DEFAULT_PAGER, os.devnull)) == 0: - pager_cmd = os.getenv(DVC_PAGER, DEFAULT_PAGER_FORMATTED) + env_pager = os.getenv(DVC_PAGER) + if env_pager: - def less_pager(text): - return pydoc.tempfilepager(pydoc.plain(text), pager_cmd) + def user_pager(text): + return pydoc.tempfilepager(pydoc.plain(text), env_pager) - return less_pager + return user_pager + else: + if os.system("({}) 2>{}".format(DEFAULT_PAGER, os.devnull)) == 0: + + def less_pager(text): + return pydoc.tempfilepager( + pydoc.plain(text), DEFAULT_PAGER_FORMATTED + ) + + return less_pager logger.warning( "Unable to find `less` in the PATH. Check out " diff --git a/tests/unit/test_dagascii.py b/tests/unit/test_dagascii.py new file mode 100644 index 0000000000..dec837006a --- /dev/null +++ b/tests/unit/test_dagascii.py @@ -0,0 +1,28 @@ +import mock +import os + +from dvc import dagascii +from dvc.env import DVC_PAGER + + +def test_less_pager_returned_when_less_found(): + with mock.patch.object(os, "system") as m: + m.return_value = 0 + pager = dagascii.find_pager() + + assert pager.__name__ == "less_pager" + + +def test_plainpager_returned_when_less_missing(): + with mock.patch.object(os, "system") as m: + m.return_value = 1 # any non-zero value + pager = dagascii.find_pager() + + assert pager.__name__ == "plainpager" + + +def test_tempfilepager_returned_when_var_defined(): + os.environ[DVC_PAGER] = dagascii.DEFAULT_PAGER + pager = dagascii.find_pager() + + assert pager.__name__ == "user_pager" From 8fb754a4f4f6a987ff1e6172b76191def8fa47d0 Mon Sep 17 00:00:00 2001 From: Tymoteusz Jankowski Date: Fri, 6 Dec 2019 20:42:10 +0100 Subject: [PATCH 10/14] Simplify if in find_pager --- dvc/dagascii.py | 23 ++++++++++------------- tests/unit/test_dagascii.py | 4 ++-- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/dvc/dagascii.py b/dvc/dagascii.py index a686392f8e..a8819bb013 100644 --- a/dvc/dagascii.py +++ b/dvc/dagascii.py @@ -28,25 +28,22 @@ def find_pager(): + def make_pager(cmd): + def pager(text): + return pydoc.tempfilepager(pydoc.plain(text), cmd) + + pager.cmd = cmd + return pager + if not sys.stdout.isatty(): return pydoc.plainpager env_pager = os.getenv(DVC_PAGER) if env_pager: + return make_pager(env_pager) - def user_pager(text): - return pydoc.tempfilepager(pydoc.plain(text), env_pager) - - return user_pager - else: - if os.system("({}) 2>{}".format(DEFAULT_PAGER, os.devnull)) == 0: - - def less_pager(text): - return pydoc.tempfilepager( - pydoc.plain(text), DEFAULT_PAGER_FORMATTED - ) - - return less_pager + if os.system("({}) 2>{}".format(DEFAULT_PAGER, os.devnull)) == 0: + return make_pager(DEFAULT_PAGER_FORMATTED) logger.warning( "Unable to find `less` in the PATH. Check out " diff --git a/tests/unit/test_dagascii.py b/tests/unit/test_dagascii.py index dec837006a..1fa61d48d4 100644 --- a/tests/unit/test_dagascii.py +++ b/tests/unit/test_dagascii.py @@ -10,7 +10,7 @@ def test_less_pager_returned_when_less_found(): m.return_value = 0 pager = dagascii.find_pager() - assert pager.__name__ == "less_pager" + assert pager.cmd == dagascii.DEFAULT_PAGER_FORMATTED def test_plainpager_returned_when_less_missing(): @@ -25,4 +25,4 @@ def test_tempfilepager_returned_when_var_defined(): os.environ[DVC_PAGER] = dagascii.DEFAULT_PAGER pager = dagascii.find_pager() - assert pager.__name__ == "user_pager" + assert pager.cmd == dagascii.DEFAULT_PAGER From 6cfc831879d3fea5c4a64b483929281a8aced726 Mon Sep 17 00:00:00 2001 From: Tymoteusz Jankowski Date: Fri, 6 Dec 2019 21:54:40 +0100 Subject: [PATCH 11/14] Use mocker fixture in test_dagascii.py --- tests/unit/test_dagascii.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/unit/test_dagascii.py b/tests/unit/test_dagascii.py index 1fa61d48d4..7ef20bd85a 100644 --- a/tests/unit/test_dagascii.py +++ b/tests/unit/test_dagascii.py @@ -5,24 +5,24 @@ from dvc.env import DVC_PAGER -def test_less_pager_returned_when_less_found(): - with mock.patch.object(os, "system") as m: - m.return_value = 0 - pager = dagascii.find_pager() +def test_less_pager_returned_when_less_found(mocker): + mocker.patch.object(os, "system", return_value=0) + + pager = dagascii.find_pager() assert pager.cmd == dagascii.DEFAULT_PAGER_FORMATTED -def test_plainpager_returned_when_less_missing(): - with mock.patch.object(os, "system") as m: - m.return_value = 1 # any non-zero value - pager = dagascii.find_pager() +def test_plainpager_returned_when_less_missing(mocker): + mocker.patch.object(os, "system", return_value=1) + + pager = dagascii.find_pager() assert pager.__name__ == "plainpager" -def test_tempfilepager_returned_when_var_defined(): - os.environ[DVC_PAGER] = dagascii.DEFAULT_PAGER +def test_tempfilepager_returned_when_var_defined(mocker): + mocker.patch.dict(os.environ, {"DVC_PAGER": dagascii.DEFAULT_PAGER}) pager = dagascii.find_pager() assert pager.cmd == dagascii.DEFAULT_PAGER From ed4e951f2259de76d1560aac8c34ef3589b31e6b Mon Sep 17 00:00:00 2001 From: Tymoteusz Jankowski Date: Fri, 6 Dec 2019 21:57:45 +0100 Subject: [PATCH 12/14] Use monkeypatch in test_dagascii.py --- tests/unit/test_dagascii.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_dagascii.py b/tests/unit/test_dagascii.py index 7ef20bd85a..6e8ad4a105 100644 --- a/tests/unit/test_dagascii.py +++ b/tests/unit/test_dagascii.py @@ -1,4 +1,3 @@ -import mock import os from dvc import dagascii @@ -21,8 +20,9 @@ def test_plainpager_returned_when_less_missing(mocker): assert pager.__name__ == "plainpager" -def test_tempfilepager_returned_when_var_defined(mocker): - mocker.patch.dict(os.environ, {"DVC_PAGER": dagascii.DEFAULT_PAGER}) +def test_tempfilepager_returned_when_var_defined(monkeypatch): + monkeypatch.setenv(DVC_PAGER, dagascii.DEFAULT_PAGER) + pager = dagascii.find_pager() assert pager.cmd == dagascii.DEFAULT_PAGER From ffefab152e7a38121e9e2a8aa8c04fcf9e08879e Mon Sep 17 00:00:00 2001 From: Tymoteusz Jankowski Date: Fri, 6 Dec 2019 22:17:44 +0100 Subject: [PATCH 13/14] Remove make_pager.cmd attribute --- dvc/dagascii.py | 12 ++++++------ tests/unit/test_dagascii.py | 18 +++++++++++------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/dvc/dagascii.py b/dvc/dagascii.py index a8819bb013..ffe7258200 100644 --- a/dvc/dagascii.py +++ b/dvc/dagascii.py @@ -27,14 +27,14 @@ ) -def find_pager(): - def make_pager(cmd): - def pager(text): - return pydoc.tempfilepager(pydoc.plain(text), cmd) +def make_pager(cmd): + def pager(text): + return pydoc.tempfilepager(pydoc.plain(text), cmd) + + return pager - pager.cmd = cmd - return pager +def find_pager(): if not sys.stdout.isatty(): return pydoc.plainpager diff --git a/tests/unit/test_dagascii.py b/tests/unit/test_dagascii.py index 6e8ad4a105..6cb23adb2c 100644 --- a/tests/unit/test_dagascii.py +++ b/tests/unit/test_dagascii.py @@ -4,15 +4,16 @@ from dvc.env import DVC_PAGER -def test_less_pager_returned_when_less_found(mocker): +def test_find_pager_uses_default_pager_when_found(mocker): + m_make_pager = mocker.patch.object(dagascii, "make_pager") mocker.patch.object(os, "system", return_value=0) - pager = dagascii.find_pager() + dagascii.find_pager() - assert pager.cmd == dagascii.DEFAULT_PAGER_FORMATTED + m_make_pager.assert_called_once_with(dagascii.DEFAULT_PAGER_FORMATTED) -def test_plainpager_returned_when_less_missing(mocker): +def test_find_pager_returns_plain_pager_when_default_missing(mocker): mocker.patch.object(os, "system", return_value=1) pager = dagascii.find_pager() @@ -20,9 +21,12 @@ def test_plainpager_returned_when_less_missing(mocker): assert pager.__name__ == "plainpager" -def test_tempfilepager_returned_when_var_defined(monkeypatch): +def test_find_pager_uses_custom_pager_when_env_var_is_defined( + mocker, monkeypatch +): + m_make_pager = mocker.patch.object(dagascii, "make_pager") monkeypatch.setenv(DVC_PAGER, dagascii.DEFAULT_PAGER) - pager = dagascii.find_pager() + dagascii.find_pager() - assert pager.cmd == dagascii.DEFAULT_PAGER + m_make_pager.assert_called_once_with(dagascii.DEFAULT_PAGER) From 44df59f96c61ec32b63c0d24d4e9a1b347adad89 Mon Sep 17 00:00:00 2001 From: Tymoteusz Jankowski Date: Fri, 6 Dec 2019 23:32:37 +0100 Subject: [PATCH 14/14] Fix test_dagascii tests --- tests/unit/test_dagascii.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/unit/test_dagascii.py b/tests/unit/test_dagascii.py index 6cb23adb2c..d9f0a205b9 100644 --- a/tests/unit/test_dagascii.py +++ b/tests/unit/test_dagascii.py @@ -1,12 +1,11 @@ -import os - from dvc import dagascii from dvc.env import DVC_PAGER def test_find_pager_uses_default_pager_when_found(mocker): + mocker.patch("sys.stdout.isatty", return_value=True) + mocker.patch("os.system", return_value=0) m_make_pager = mocker.patch.object(dagascii, "make_pager") - mocker.patch.object(os, "system", return_value=0) dagascii.find_pager() @@ -14,7 +13,8 @@ def test_find_pager_uses_default_pager_when_found(mocker): def test_find_pager_returns_plain_pager_when_default_missing(mocker): - mocker.patch.object(os, "system", return_value=1) + mocker.patch("sys.stdout.isatty", return_value=True) + mocker.patch("os.system", return_value=1) pager = dagascii.find_pager() @@ -24,9 +24,18 @@ def test_find_pager_returns_plain_pager_when_default_missing(mocker): def test_find_pager_uses_custom_pager_when_env_var_is_defined( mocker, monkeypatch ): + mocker.patch("sys.stdout.isatty", return_value=True) m_make_pager = mocker.patch.object(dagascii, "make_pager") monkeypatch.setenv(DVC_PAGER, dagascii.DEFAULT_PAGER) dagascii.find_pager() m_make_pager.assert_called_once_with(dagascii.DEFAULT_PAGER) + + +def test_find_pager_returns_plain_pager_when_is_not_atty(mocker): + mocker.patch("sys.stdout.isatty", return_value=False) + + pager = dagascii.find_pager() + + assert pager.__name__ == "plainpager"