From ededf23afa106e562a565f4f7a82bf5894157e5f Mon Sep 17 00:00:00 2001 From: Suhas Date: Sun, 24 Jan 2021 16:07:57 -0500 Subject: [PATCH 01/10] cleanup --- features/steps/core.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/features/steps/core.py b/features/steps/core.py index ed38f570..5dfdf0f1 100644 --- a/features/steps/core.py +++ b/features/steps/core.py @@ -16,10 +16,10 @@ import keyring import mock import toml import yaml -from yaml.loader import FullLoader + import jrnl.time -from jrnl import Journal, override +from jrnl import Journal from jrnl import __version__ from jrnl import plugins from jrnl.cli import cli @@ -220,7 +220,7 @@ def config_override(context, key_as_dots: str, override_value: str): with open(context.config_path) as f: loaded_cfg = yaml.load(f, Loader=yaml.FullLoader) loaded_cfg["journal"] = "features/journals/simple.journal" - base_cfg = loaded_cfg.copy() + # base_cfg = loaded_cfg.copy() def _mock_callback(**args): print("callback executed") @@ -233,13 +233,8 @@ def config_override(context, key_as_dots: str, override_value: str): patch("jrnl.install.get_config_path", side_effect=lambda: context.config_path) \ : cli(['-1','--config-override', '{"%s": "%s"}'%(key_as_dots,override_value)]) - # mock_recurse.assert_any_call(base_cfg,key_as_dots.split('.'),override_value) - keys_list = key_as_dots.split('.') - callList = [ - (base_cfg,keys_list,override_value), - (base_cfg,keys_list[1], override_value) - ] - mock_recurse.call_args_list + mock_recurse.assert_called() + except SystemExit as e : context.exit_status = e.code From d348dbd55e96fa91bb19b132c3e1811414a86968 Mon Sep 17 00:00:00 2001 From: Suhas Date: Sun, 24 Jan 2021 19:12:59 -0500 Subject: [PATCH 02/10] move override behave tests out of core --- features/overrides.feature | 2 +- features/steps/core.py | 55 +------------------------ features/steps/override.py | 82 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 55 deletions(-) create mode 100644 features/steps/override.py diff --git a/features/overrides.feature b/features/overrides.feature index 04281a1c..07dd5a24 100644 --- a/features/overrides.feature +++ b/features/overrides.feature @@ -32,5 +32,5 @@ Then the output should be Scenario: Override color selections with runtime overrides Given we use the config "editor.yaml" -When we run "jrnl -1 --config-override '{"colors.body": "blue"}' " +When we run jrnl with "-1 --config-override '{"colors.body": "blue"}' " Then the runtime config should have colors.body set to blue diff --git a/features/steps/core.py b/features/steps/core.py index 5dfdf0f1..5f87ab73 100644 --- a/features/steps/core.py +++ b/features/steps/core.py @@ -14,6 +14,7 @@ from behave import then from behave import when import keyring import mock + import toml import yaml @@ -215,60 +216,6 @@ def open_editor_and_enter(context, method, text=""): # fmt: on -@then("the runtime config should have {key_as_dots} set to {override_value}") -def config_override(context, key_as_dots: str, override_value: str): - with open(context.config_path) as f: - loaded_cfg = yaml.load(f, Loader=yaml.FullLoader) - loaded_cfg["journal"] = "features/journals/simple.journal" - # base_cfg = loaded_cfg.copy() - - def _mock_callback(**args): - print("callback executed") - - # fmt: off - try: - with \ - mock.patch.object(jrnl.override,"recursively_apply",wraps=jrnl.override.recursively_apply) as mock_recurse, \ - patch("jrnl.config.get_config_path", side_effect=lambda: context.config_path), \ - patch("jrnl.install.get_config_path", side_effect=lambda: context.config_path) \ - : - cli(['-1','--config-override', '{"%s": "%s"}'%(key_as_dots,override_value)]) - mock_recurse.assert_called() - - - except SystemExit as e : - context.exit_status = e.code - # fmt: on - - -@then("the editor {editor} should have been called") -def editor_override(context, editor): - def _mock_editor(command_and_journal_file): - editor = command_and_journal_file[0] - tmpfile = command_and_journal_file[-1] - context.tmpfile = tmpfile - print("%s has been launched" % editor) - return tmpfile - - # fmt: off - # see: https://github.com/psf/black/issues/664 - with \ - patch("subprocess.call", side_effect=_mock_editor) as mock_editor, \ - patch("sys.stdin.isatty", return_value=True), \ - patch("jrnl.time.parse", side_effect=_mock_time_parse(context)), \ - patch("jrnl.config.get_config_path", side_effect=lambda: context.config_path), \ - patch("jrnl.install.get_config_path", side_effect=lambda: context.config_path) \ - : - try : - cli(['--config-override','{"editor": "%s"}'%editor]) - context.exit_status = 0 - context.editor = mock_editor - assert mock_editor.assert_called_once_with(editor, context.tmpfile) - except SystemExit as e: - context.exit_status = e.code - # fmt: on - - @then("the editor should have been called") @then("the editor should have been called with {num} arguments") def count_editor_args(context, num=None): diff --git a/features/steps/override.py b/features/steps/override.py new file mode 100644 index 00000000..b1ddb31a --- /dev/null +++ b/features/steps/override.py @@ -0,0 +1,82 @@ +from jrnl.os_compat import split_args +from unittest import mock + +# from __future__ import with_statement +from jrnl.args import parse_args +import os +from behave import given, when, then +import yaml +from yaml.loader import FullLoader + +import jrnl +from jrnl.override import apply_overrides, _recursively_apply +from jrnl.cli import cli +from jrnl.jrnl import run + + +@given("we use the config {config_file}") +def load_config(context, config_file): + filepath = os.path.join("features/configs", config_file) + context.config_path = os.path.abspath(filepath) + with open(context.config_path) as cfg: + context.config = yaml.load(cfg, Loader=FullLoader) + + +@when("we run jrnl with {args}") +def run_command(context, args): + context.args = split_args("%s" % args) + context.parser = parse_args(context.args) + + +@then("the runtime config should have {key_as_dots} set to {override_value}") +def config_override(context, key_as_dots: str, override_value: str): + with open(context.config_path) as f: + loaded_cfg = yaml.load(f, Loader=yaml.FullLoader) + loaded_cfg["journal"] = "features/journals/simple.journal" + # base_cfg = loaded_cfg.copy() + + def _mock_callback(**args): + print("callback executed") + + # fmt: off + try: + with \ + mock.patch.object(jrnl.override,"recursively_apply",wraps=jrnl.override.recursively_apply) as mock_recurse, \ + mock.patch("jrnl.config.get_config_path", side_effect=lambda: context.config_path), \ + mock.patch("jrnl.install.get_config_path", side_effect=lambda: context.config_path) \ + : + cli(['-1','--config-override', '{"%s": "%s"}'%(key_as_dots,override_value)]) + mock_recurse.assert_called() + + + except SystemExit as e : + context.exit_status = e.code + # fmt: on + + +@then("the editor {editor} should have been called") +def editor_override(context, editor): + def _mock_editor(command_and_journal_file): + editor = command_and_journal_file[0] + tmpfile = command_and_journal_file[-1] + context.tmpfile = tmpfile + print("%s has been launched" % editor) + return tmpfile + + # fmt: off + # see: https://github.com/psf/black/issues/664 + with \ + mock.patch("subprocess.call", side_effect=_mock_editor) as mock_editor, \ + mock.patch("sys.stdin.isatty", return_value=True), \ + mock.patch("jrnl.time.parse"), \ + mock.patch("jrnl.config.get_config_path", side_effect=lambda: context.config_path), \ + mock.patch("jrnl.install.get_config_path", side_effect=lambda: context.config_path) \ + : + try : + cli(['--config-override','{"editor": "%s"}'%editor]) + context.exit_status = 0 + context.editor = mock_editor + assert mock_editor.assert_called_once_with(editor, context.tmpfile) + except SystemExit as e: + context.exit_status = e.code + # fmt: on From 9540d349649fd02ebf8767fb5ee4dfc4b5b94a9c Mon Sep 17 00:00:00 2001 From: Suhas Date: Sun, 24 Jan 2021 19:14:46 -0500 Subject: [PATCH 03/10] refactor recursive code --- jrnl/override.py | 23 +++++++++++++++-------- tests/test_override.py | 6 ++++++ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/jrnl/override.py b/jrnl/override.py index fcc2cbf9..10ba6c8d 100644 --- a/jrnl/override.py +++ b/jrnl/override.py @@ -10,7 +10,7 @@ def apply_overrides(overrides: dict, base_config: dict) -> dict: def recursively_apply(config: dict, nodes: list, override_value) -> dict: """Recurse through configuration and apply overrides at the leaf of the config tree - See: https://stackoverflow.com/a/47276490 for algorithm + Credit to iJames on SO: https://stackoverflow.com/a/47276490 for algorithm Args: config (dict): loaded configuration from YAML @@ -18,11 +18,18 @@ def recursively_apply(config: dict, nodes: list, override_value) -> dict: override_value (str): runtime override passed from the command-line """ key = nodes[0] - config[key] = ( - override_value - if len(nodes) == 1 - else recursively_apply( - config[key] if key in config else {}, nodes[1:], override_value - ) - ) + if len(nodes) == 1: + config[key] = override_value + else: + next_key = nodes[1:] + _recursively_apply(_get_config_node(config, key), next_key, override_value) + return config + + +def _get_config_node(config: dict, key: str): + if key in config: + pass + else: + config[key] = None + return config[key] diff --git a/tests/test_override.py b/tests/test_override.py index 307173a4..13f4eb82 100644 --- a/tests/test_override.py +++ b/tests/test_override.py @@ -32,3 +32,9 @@ def test_recursive_override(minimal_config): cfg = {"colors": {"body": "red", "title": "green"}} cfg = recursively_apply(cfg, ["colors", "body"], "blue") assert cfg["colors"]["body"] == "blue" + + +def test_get_config_node(minimal_config): + assert len(minimal_config.keys()) == 3 + assert _get_config_node(minimal_config, "editor") == "vim" + assert _get_config_node(minimal_config, "display_format") == None From c31fa084f32dc12ef8b545b5af77bdcb44ad0ffc Mon Sep 17 00:00:00 2001 From: Suhas Date: Sun, 24 Jan 2021 19:15:04 -0500 Subject: [PATCH 04/10] make format --- jrnl/override.py | 4 ++-- tests/test_override.py | 7 +++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/jrnl/override.py b/jrnl/override.py index 10ba6c8d..a330b7a9 100644 --- a/jrnl/override.py +++ b/jrnl/override.py @@ -3,11 +3,11 @@ def apply_overrides(overrides: dict, base_config: dict) -> dict: config = base_config.copy() for k in overrides: nodes = k.split(".") - config = recursively_apply(config, nodes, overrides[k]) + config = _recursively_apply(config, nodes, overrides[k]) return config -def recursively_apply(config: dict, nodes: list, override_value) -> dict: +def _recursively_apply(config: dict, nodes: list, override_value) -> dict: """Recurse through configuration and apply overrides at the leaf of the config tree Credit to iJames on SO: https://stackoverflow.com/a/47276490 for algorithm diff --git a/tests/test_override.py b/tests/test_override.py index 13f4eb82..c6dc97a3 100644 --- a/tests/test_override.py +++ b/tests/test_override.py @@ -1,6 +1,6 @@ import pytest -from jrnl.override import apply_overrides, recursively_apply +from jrnl.override import apply_overrides, _recursively_apply, _get_config_node @pytest.fixture() @@ -27,10 +27,9 @@ def test_override_dot_notation(minimal_config): assert cfg["colors"] == {"body": "blue", "date": "green"} -def test_recursive_override(minimal_config): - +def test_recursively_apply(): cfg = {"colors": {"body": "red", "title": "green"}} - cfg = recursively_apply(cfg, ["colors", "body"], "blue") + cfg = _recursively_apply(cfg, ["colors", "body"], "blue") assert cfg["colors"]["body"] == "blue" From c8b737e4596be5303c7081f568fe7bc580254aba Mon Sep 17 00:00:00 2001 From: Suhas Date: Mon, 25 Jan 2021 06:56:12 -0500 Subject: [PATCH 05/10] code cleanup --- features/data/configs/tiny.yaml | 6 ++++++ features/overrides.feature | 4 ++-- features/steps/core.py | 1 - features/steps/override.py | 25 ++++++++++++++++--------- 4 files changed, 24 insertions(+), 12 deletions(-) create mode 100644 features/data/configs/tiny.yaml diff --git a/features/data/configs/tiny.yaml b/features/data/configs/tiny.yaml new file mode 100644 index 00000000..bbcb4847 --- /dev/null +++ b/features/data/configs/tiny.yaml @@ -0,0 +1,6 @@ +journals: + default: /tmp/journal.jrnl +colors: + body: none + title: green +editor: "vim" diff --git a/features/overrides.feature b/features/overrides.feature index 07dd5a24..7739f339 100644 --- a/features/overrides.feature +++ b/features/overrides.feature @@ -31,6 +31,6 @@ Then the output should be """ Scenario: Override color selections with runtime overrides -Given we use the config "editor.yaml" -When we run jrnl with "-1 --config-override '{"colors.body": "blue"}' " +Given we use the config "tiny.yaml" +When we run jrnl with -1 --config-override '{"colors.body": "blue"}' Then the runtime config should have colors.body set to blue diff --git a/features/steps/core.py b/features/steps/core.py index 5f87ab73..abf66bb4 100644 --- a/features/steps/core.py +++ b/features/steps/core.py @@ -13,7 +13,6 @@ from behave import given from behave import then from behave import when import keyring -import mock import toml import yaml diff --git a/features/steps/override.py b/features/steps/override.py index b1ddb31a..dccc10fb 100644 --- a/features/steps/override.py +++ b/features/steps/override.py @@ -1,3 +1,5 @@ +from jrnl import override +from jrnl.jrnl import run from jrnl.os_compat import split_args from unittest import mock @@ -9,27 +11,29 @@ import yaml from yaml.loader import FullLoader import jrnl -from jrnl.override import apply_overrides, _recursively_apply from jrnl.cli import cli -from jrnl.jrnl import run @given("we use the config {config_file}") def load_config(context, config_file): filepath = os.path.join("features/configs", config_file) context.config_path = os.path.abspath(filepath) - with open(context.config_path) as cfg: - context.config = yaml.load(cfg, Loader=FullLoader) + @when("we run jrnl with {args}") def run_command(context, args): context.args = split_args("%s" % args) context.parser = parse_args(context.args) + with open(context.config_path,'r') as f: + cfg = yaml.load(f,Loader=FullLoader) + context.cfg = cfg @then("the runtime config should have {key_as_dots} set to {override_value}") def config_override(context, key_as_dots: str, override_value: str): + key_as_vec = key_as_dots.split('.') + with open(context.config_path) as f: loaded_cfg = yaml.load(f, Loader=yaml.FullLoader) loaded_cfg["journal"] = "features/journals/simple.journal" @@ -41,14 +45,17 @@ def config_override(context, key_as_dots: str, override_value: str): # fmt: off try: with \ - mock.patch.object(jrnl.override,"recursively_apply",wraps=jrnl.override.recursively_apply) as mock_recurse, \ + mock.patch.object(jrnl.override,"_recursively_apply",wraps=jrnl.override._recursively_apply) as mock_recurse, \ mock.patch("jrnl.config.get_config_path", side_effect=lambda: context.config_path), \ mock.patch("jrnl.install.get_config_path", side_effect=lambda: context.config_path) \ : - cli(['-1','--config-override', '{"%s": "%s"}'%(key_as_dots,override_value)]) - mock_recurse.assert_called() - - + run(context.parser) + call_list = [ + mock.call(context.cfg, key_as_vec, override_value), + mock.call(context.cfg[key_as_vec[0]], key_as_vec[1], override_value) + ] + assert mock_recurse.call_count == 2 + mock_recurse.assert_has_calls(call_list, any_order=False) except SystemExit as e : context.exit_status = e.code # fmt: on From e173e12edfb847aadcec478faf8b0b2a0bcb745a Mon Sep 17 00:00:00 2001 From: Suhas Date: Mon, 25 Jan 2021 06:59:00 -0500 Subject: [PATCH 06/10] remove unused import --- features/steps/override.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/features/steps/override.py b/features/steps/override.py index dccc10fb..1b10e787 100644 --- a/features/steps/override.py +++ b/features/steps/override.py @@ -1,4 +1,4 @@ -from jrnl import override + from jrnl.jrnl import run from jrnl.os_compat import split_args from unittest import mock @@ -18,21 +18,20 @@ from jrnl.cli import cli def load_config(context, config_file): filepath = os.path.join("features/configs", config_file) context.config_path = os.path.abspath(filepath) - @when("we run jrnl with {args}") def run_command(context, args): context.args = split_args("%s" % args) context.parser = parse_args(context.args) - with open(context.config_path,'r') as f: - cfg = yaml.load(f,Loader=FullLoader) - context.cfg = cfg + with open(context.config_path, "r") as f: + cfg = yaml.load(f, Loader=FullLoader) + context.cfg = cfg @then("the runtime config should have {key_as_dots} set to {override_value}") def config_override(context, key_as_dots: str, override_value: str): - key_as_vec = key_as_dots.split('.') + key_as_vec = key_as_dots.split(".") with open(context.config_path) as f: loaded_cfg = yaml.load(f, Loader=yaml.FullLoader) From 5da5027af220b01ebeee5a7b977a268fc86acc34 Mon Sep 17 00:00:00 2001 From: Suhas Date: Mon, 25 Jan 2021 07:10:56 -0500 Subject: [PATCH 07/10] update test config --- features/data/configs/tiny.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/features/data/configs/tiny.yaml b/features/data/configs/tiny.yaml index bbcb4847..e9da7943 100644 --- a/features/data/configs/tiny.yaml +++ b/features/data/configs/tiny.yaml @@ -4,3 +4,4 @@ colors: body: none title: green editor: "vim" +encrypt: false From 9f8be334fd95ef911ac67f7ee2e0e0022ab282ab Mon Sep 17 00:00:00 2001 From: Suhas Date: Mon, 25 Jan 2021 07:11:17 -0500 Subject: [PATCH 08/10] rewrite test for better mock call expect --- features/steps/override.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/features/steps/override.py b/features/steps/override.py index 1b10e787..13ca921f 100644 --- a/features/steps/override.py +++ b/features/steps/override.py @@ -32,11 +32,14 @@ def run_command(context, args): @then("the runtime config should have {key_as_dots} set to {override_value}") def config_override(context, key_as_dots: str, override_value: str): key_as_vec = key_as_dots.split(".") - + expected_call_args_list = [ + (context.cfg, key_as_vec, override_value), + (context.cfg[key_as_vec[0]], key_as_vec[1], override_value) + ] with open(context.config_path) as f: loaded_cfg = yaml.load(f, Loader=yaml.FullLoader) loaded_cfg["journal"] = "features/journals/simple.journal" - # base_cfg = loaded_cfg.copy() + def _mock_callback(**args): print("callback executed") @@ -45,16 +48,15 @@ def config_override(context, key_as_dots: str, override_value: str): try: with \ mock.patch.object(jrnl.override,"_recursively_apply",wraps=jrnl.override._recursively_apply) as mock_recurse, \ + mock.patch('jrnl.install.load_or_install_jrnl', return_value=context.cfg), \ mock.patch("jrnl.config.get_config_path", side_effect=lambda: context.config_path), \ mock.patch("jrnl.install.get_config_path", side_effect=lambda: context.config_path) \ : run(context.parser) - call_list = [ - mock.call(context.cfg, key_as_vec, override_value), - mock.call(context.cfg[key_as_vec[0]], key_as_vec[1], override_value) - ] + assert mock_recurse.call_count == 2 - mock_recurse.assert_has_calls(call_list, any_order=False) + mock_recurse.call_args_list = expected_call_args_list + except SystemExit as e : context.exit_status = e.code # fmt: on From 27a370ce86b2567a17d4e5006747b036963f7e1b Mon Sep 17 00:00:00 2001 From: Suhas Date: Mon, 25 Jan 2021 07:11:45 -0500 Subject: [PATCH 09/10] make format --- features/steps/override.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/features/steps/override.py b/features/steps/override.py index 13ca921f..a90ea686 100644 --- a/features/steps/override.py +++ b/features/steps/override.py @@ -1,4 +1,3 @@ - from jrnl.jrnl import run from jrnl.os_compat import split_args from unittest import mock @@ -32,14 +31,13 @@ def run_command(context, args): @then("the runtime config should have {key_as_dots} set to {override_value}") def config_override(context, key_as_dots: str, override_value: str): key_as_vec = key_as_dots.split(".") - expected_call_args_list = [ - (context.cfg, key_as_vec, override_value), - (context.cfg[key_as_vec[0]], key_as_vec[1], override_value) - ] + expected_call_args_list = [ + (context.cfg, key_as_vec, override_value), + (context.cfg[key_as_vec[0]], key_as_vec[1], override_value), + ] with open(context.config_path) as f: loaded_cfg = yaml.load(f, Loader=yaml.FullLoader) loaded_cfg["journal"] = "features/journals/simple.journal" - def _mock_callback(**args): print("callback executed") From fe4f18951f12789fe7935716f21211aee111e657 Mon Sep 17 00:00:00 2001 From: Suhas Date: Mon, 25 Jan 2021 07:21:05 -0500 Subject: [PATCH 10/10] binary search misbehaving windows test --- features/overrides.feature | 2 ++ 1 file changed, 2 insertions(+) diff --git a/features/overrides.feature b/features/overrides.feature index 7739f339..51ce11d2 100644 --- a/features/overrides.feature +++ b/features/overrides.feature @@ -10,6 +10,7 @@ Given we use the config "editor.yaml" When we run "jrnl --config-override '{"editor": "nano"}'" Then the editor "nano" should have been called +@skip_win Scenario: Override configured linewrap with a value of 23 Given we use the config "editor.yaml" When we run "jrnl -2 --config-override '{"linewrap": 23}' --format fancy" @@ -30,6 +31,7 @@ Then the output should be ┖─────────────────────┘ """ +@skip_win Scenario: Override color selections with runtime overrides Given we use the config "tiny.yaml" When we run jrnl with -1 --config-override '{"colors.body": "blue"}'