Improve handling of mocking logic in pytest (#1382)

* WIP

* fix handling of user input (stdin, input, getpass)

* take out redundant pytest step

* fix handling of 'we should' statements

* fix test that doesn't use a config file

* fix another test that uses stdin

Co-authored-by: Micah Jerome Ellison <micah.jerome.ellison@gmail.com>

* remove .tool-versions file per PR feedback

* add comment to clarify why disembodied variables are here

Co-authored-by: Micah Jerome Ellison <micah.jerome.ellison@gmail.com>
This commit is contained in:
Jonathan Wren 2021-12-11 12:35:32 -08:00 committed by GitHub
parent 3518e37087
commit 2ab485de8c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
16 changed files with 259 additions and 197 deletions

View file

@ -8,17 +8,19 @@ import tempfile
from keyring import backend
from keyring import errors
from keyring import set_keyring
from pytest import fixture
from unittest.mock import patch
from .helpers import get_fixture
import toml
from jrnl.config import load_config
from jrnl.os_compat import split_args
# --- Keyring --- #
@fixture
def keyring():
set_keyring(NoKeyring())
return NoKeyring()
@fixture
@ -75,13 +77,90 @@ class FailedKeyring(backend.KeyringBackend):
# ----- Misc ----- #
@fixture
def cli_run():
return {"status": 0, "stdout": None, "stderr": None}
def cli_run(
mock_factories,
mock_args,
mock_is_tty,
mock_config_path,
mock_editor,
mock_user_input,
mock_overrides,
mock_password,
):
# Check if we need more mocks
mock_factories.update(mock_args)
mock_factories.update(mock_is_tty)
mock_factories.update(mock_overrides)
mock_factories.update(mock_editor)
mock_factories.update(mock_config_path)
mock_factories.update(mock_user_input)
mock_factories.update(mock_password)
return {
"status": 0,
"stdout": None,
"stderr": None,
"mocks": {},
"mock_factories": mock_factories,
}
@fixture
def mocks():
return dict()
def mock_factories():
return {}
@fixture
def mock_args(cache_dir, request):
def _mock_args():
command = get_fixture(request, "command", "")
if cache_dir["exists"]:
command = command.format(cache_dir=cache_dir["path"])
args = split_args(command)
return patch("sys.argv", ["jrnl"] + args)
return {"args": _mock_args}
@fixture
def mock_is_tty(is_tty):
return {"is_tty": lambda: patch("sys.stdin.isatty", return_value=is_tty)}
@fixture
def mock_overrides(config_in_memory):
from jrnl.override import apply_overrides
def my_overrides(*args, **kwargs):
result = apply_overrides(*args, **kwargs)
config_in_memory["overrides"] = result
return result
return {
"overrides": lambda: patch(
"jrnl.jrnl.apply_overrides", side_effect=my_overrides
)
}
@fixture
def mock_config_path(request):
config_path = get_fixture(request, "config_path")
if not config_path:
return {}
return {
"config_path_install": lambda: patch(
"jrnl.install.get_config_path", return_value=config_path
),
"config_path_config": lambda: patch(
"jrnl.config.get_config_path", return_value=config_path
),
}
@fixture
@ -94,12 +173,6 @@ def working_dir(request):
return os.path.join(request.config.rootpath, "tests")
@fixture
def config_path(temp_dir):
os.chdir(temp_dir.name)
return temp_dir.name + "/jrnl.yaml"
@fixture
def toml_version(working_dir):
pyproject = os.path.join(working_dir, "..", "pyproject.toml")
@ -108,8 +181,23 @@ def toml_version(working_dir):
@fixture
def password():
return ""
def mock_password(request):
def _mock_password():
password = get_fixture(request, "password")
user_input = get_fixture(request, "user_input")
if password:
password = password.splitlines()
elif user_input:
password = user_input.splitlines()
if not password:
password = Exception("Unexpected call for password")
return patch("getpass.getpass", side_effect=password)
return {"getpass": _mock_password}
@fixture
@ -127,19 +215,36 @@ def str_value():
return ""
@fixture
def command():
return ""
@fixture
def should_not():
return False
@fixture
def user_input():
return ""
def mock_user_input(request, is_tty):
def _generator(target):
def _mock_user_input():
user_input = get_fixture(request, "user_input", None)
if user_input is None:
user_input = Exception("Unexpected call for user input")
else:
user_input = user_input.splitlines() if is_tty else [user_input]
return patch(target, side_effect=user_input)
return _mock_user_input
return {
"stdin": _generator("sys.stdin.read"),
"input": _generator("builtins.input"),
}
@fixture
def is_tty(input_method):
assert input_method in ["", "enter", "pipe"]
return input_method != "pipe"
@fixture
@ -187,7 +292,7 @@ def editor_state():
@fixture
def editor(editor_state):
def mock_editor(editor_state):
def _mock_editor(editor_command):
tmpfile = editor_command[-1]
@ -203,4 +308,4 @@ def editor(editor_state):
file_content = f.read()
editor_state["tmpfile"]["content"] = file_content
return _mock_editor
return {"editor": lambda: patch("subprocess.call", side_effect=_mock_editor)}

View file

@ -11,7 +11,6 @@ from unittest.mock import MagicMock
from unittest.mock import patch
from xml.etree import ElementTree
from keyring import set_keyring
from pytest_bdd import given
from pytest_bdd.parsers import parse
@ -20,6 +19,7 @@ from jrnl.time import __get_pdt_calendar
from .fixtures import FailedKeyring
from .fixtures import TestKeyring
from .helpers import get_fixture
@given(parse("we {editor_method} to the editor if opened\n{editor_input}"))
@ -36,9 +36,8 @@ def we_enter_editor(editor_method, editor_input, editor_state):
editor_state["intent"] = {"method": file_method, "input": editor_input}
@given(parse('now is "<date_str>"'))
@given(parse('now is "{date_str}"'))
def now_is_str(date_str, mocks):
def now_is_str(date_str, mock_factories):
class DatetimeMagicMock(MagicMock):
# needed because jrnl does some reflection on datetime
def __instancecheck__(self, subclass):
@ -63,8 +62,8 @@ def now_is_str(date_str, mocks):
date_str_input, mocked_now()
)
mocks["datetime"] = patch("datetime.datetime", new=datetime_mock)
mocks["calendar_parse"] = patch(
mock_factories["datetime"] = lambda: patch("datetime.datetime", new=datetime_mock)
mock_factories["calendar_parse"] = lambda: patch(
"jrnl.time.__get_pdt_calendar", return_value=calendar_mock
)
@ -73,17 +72,22 @@ def now_is_str(date_str, mocks):
@given(parse("we have a {keyring_type} keyring"), target_fixture="keyring")
def we_have_type_of_keyring(keyring_type):
if keyring_type == "failed":
set_keyring(FailedKeyring())
return FailedKeyring()
else:
set_keyring(TestKeyring())
return TestKeyring()
@given(parse('we use the config "{config_file}"'), target_fixture="config_path")
@given('we use the config "<config_file>"', target_fixture="config_path")
def we_use_the_config(config_file, temp_dir, working_dir):
@given(parse("we use no config"), target_fixture="config_path")
def we_use_the_config(request, temp_dir, working_dir):
config_file = get_fixture(request, "config_file")
# Move into temp dir as cwd
os.chdir(temp_dir.name)
if not config_file:
return os.path.join(temp_dir.name, "non_existing_config.yaml")
# Copy the config file over
config_source = os.path.join(working_dir, "data", "configs", config_file)
config_dest = os.path.join(temp_dir.name, config_file)
@ -106,7 +110,6 @@ def we_use_the_config(config_file, temp_dir, working_dir):
@given(parse('the config "{config_file}" exists'), target_fixture="config_path")
@given('the config "<config_file>" exists', target_fixture="config_path")
def config_exists(config_file, temp_dir, working_dir):
config_source = os.path.join(working_dir, "data", "configs", config_file)
config_dest = os.path.join(temp_dir.name, config_file)

View file

@ -49,3 +49,24 @@ def get_nested_val(dictionary, path, *default):
if default:
return default[0]
raise
# @see: https://stackoverflow.com/a/41599695/569146
def spy_wrapper(wrapped_function):
from unittest import mock
mock = mock.MagicMock()
def wrapper(self, *args, **kwargs):
mock(*args, **kwargs)
return wrapped_function(self, *args, **kwargs)
wrapper.mock = mock
return wrapper
def get_fixture(request, name, default=None):
result = default
if name in request.node.fixturenames:
result = request.getfixturevalue(name)
return result

View file

@ -30,38 +30,47 @@ def output_should_match(regex, cli_run):
assert matches, f"\nRegex didn't match:\n{regex}\n{str(out)}\n{str(matches)}"
@then(parse("the output should contain\n{expected_output}"))
@then(parse('the output should contain "{expected_output}"'))
@then('the output should contain "<expected_output>"')
@then(parse("the {which_output_stream} output should contain\n{expected_output}"))
@then(parse('the {which_output_stream} output should contain "{expected_output}"'))
def output_should_contain(expected_output, which_output_stream, cli_run):
@then(parse("the output {should_or_should_not} contain\n{expected_output}"))
@then(parse('the output {should_or_should_not} contain "{expected_output}"'))
@then(
parse(
"the {which_output_stream} output {should_or_should_not} contain\n{expected_output}"
)
)
@then(
parse(
'the {which_output_stream} output {should_or_should_not} contain "{expected_output}"'
)
)
def output_should_contain(
expected_output, which_output_stream, cli_run, should_or_should_not
):
we_should = parse_should_or_should_not(should_or_should_not)
assert expected_output
if which_output_stream is None:
assert (expected_output in cli_run["stdout"]) or (
expected_output in cli_run["stderr"]
assert ((expected_output in cli_run["stdout"]) == we_should) or (
(expected_output in cli_run["stderr"]) == we_should
)
elif which_output_stream == "standard":
assert expected_output in cli_run["stdout"]
assert (expected_output in cli_run["stdout"]) == we_should
elif which_output_stream == "error":
assert expected_output in cli_run["stderr"]
assert (expected_output in cli_run["stderr"]) == we_should
else:
assert expected_output in cli_run[which_output_stream]
assert (expected_output in cli_run[which_output_stream]) == we_should
@then(parse("the output should not contain\n{expected_output}"))
@then(parse('the output should not contain "{expected_output}"'))
@then('the output should not contain "<expected_output>"')
def output_should_not_contain(expected_output, cli_run):
assert expected_output not in cli_run["stdout"]
@then(parse("the output should be\n{expected_output}"))
@then(parse('the output should be "{expected_output}"'))
@then('the output should be "<expected_output>"')
def output_should_be(expected_output, cli_run):
actual = cli_run["stdout"].strip()
expected = expected_output.strip()
@ -75,7 +84,6 @@ def output_should_be_empty(cli_run):
@then(parse('the output should contain the date "{date}"'))
@then('the output should contain the date "<date>"')
def output_should_contain_date(date, cli_run):
assert date and date in cli_run["stdout"]
@ -94,12 +102,6 @@ def output_should_be_columns_wide(cli_run, width):
assert len(line) <= width
@then(parse('we should see the message "{text}"'))
def should_see_the_message(text, cli_run):
out = cli_run["stderr"]
assert text in out, [text, out]
@then(
parse(
'the config for journal "{journal_name}" {should_or_should_not} contain "{some_yaml}"'
@ -126,10 +128,7 @@ def config_var_on_disk(config_on_disk, journal_name, should_or_should_not, some_
# `expected` objects formatted in yaml only compare one level deep
actual_slice = {key: actual.get(key, None) for key in expected.keys()}
if we_should:
assert expected == actual_slice
else:
assert expected != actual_slice
assert (expected == actual_slice) == we_should
@then(
@ -160,10 +159,7 @@ def config_var_in_memory(
# `expected` objects formatted in yaml only compare one level deep
actual_slice = {key: get_nested_val(actual, key) for key in expected.keys()}
if we_should:
assert expected == actual_slice
else:
assert expected != actual_slice
assert (expected == actual_slice) == we_should
@then("we should be prompted for a password")
@ -355,10 +351,7 @@ def count_elements(number, item, cli_run):
def count_editor_args(num_args, cli_run, editor_state, should_or_should_not):
we_should = parse_should_or_should_not(should_or_should_not)
if we_should:
assert cli_run["mocks"]["editor"].called
else:
assert not cli_run["mocks"]["editor"].called
assert cli_run["mocks"]["editor"].called == we_should
if isinstance(num_args, int):
assert len(editor_state["command"]) == int(num_args)
@ -368,10 +361,7 @@ def count_editor_args(num_args, cli_run, editor_state, should_or_should_not):
def stdin_prompt_called(cli_run, should_or_should_not):
we_should = parse_should_or_should_not(should_or_should_not)
if we_should:
assert cli_run["mocks"]["stdin"].called
else:
assert not cli_run["mocks"]["stdin"].called
assert cli_run["mocks"]["stdin"].called == we_should
@then(parse('the editor filename should end with "{suffix}"'))

View file

@ -3,14 +3,12 @@
from contextlib import ExitStack
import os
from unittest.mock import patch
from pytest_bdd import parsers
from pytest_bdd import when
from pytest_bdd.parsers import parse
from pytest_bdd.parsers import re
from jrnl.cli import cli
from jrnl.os_compat import split_args
@when(parse('we change directory to "{directory_name}"'))
@ -21,103 +19,38 @@ def when_we_change_directory(directory_name):
os.chdir(directory_name)
# These variables are used in the `@when(re(...))` section below
command = '(?P<command>[^"]+)'
input_method = "(?P<input_method>enter|pipe)"
user_input = '(?P<user_input>[^"]+)'
@when(parse('we run "jrnl {command}" and {input_method}\n{user_input}'))
@when(
parsers.re(
'we run "jrnl (?P<command>[^"]+)" and (?P<input_method>enter|pipe) "(?P<user_input>[^"]+)"'
)
)
@when(parse('we run "jrnl" and {input_method} "{user_input}"'))
@when(re(f'we run "jrnl {command}" and {input_method} "{user_input}"'))
@when(re(f'we run "jrnl" and {input_method} "{user_input}"'))
@when(parse('we run "jrnl {command}"'))
@when('we run "jrnl <command>"')
@when('we run "jrnl"')
def we_run(
command,
config_path,
config_in_memory,
user_input,
cli_run,
capsys,
password,
cache_dir,
editor,
keyring,
input_method,
mocks,
):
assert input_method in ["", "enter", "pipe"]
is_tty = input_method != "pipe"
def we_run_jrnl(cli_run, capsys, keyring):
from keyring import set_keyring
if cache_dir["exists"]:
command = command.format(cache_dir=cache_dir["path"])
args = split_args(command)
status = 0
if user_input:
user_input = user_input.splitlines() if is_tty else [user_input]
if password:
password = password.splitlines()
if not password and user_input:
password = user_input
set_keyring(keyring)
with ExitStack() as stack:
# Always mock
from jrnl.override import apply_overrides
mocks = cli_run["mocks"]
factories = cli_run["mock_factories"]
def my_overrides(*args, **kwargs):
result = apply_overrides(*args, **kwargs)
config_in_memory["overrides"] = result
return result
stack.enter_context(
patch("jrnl.jrnl.apply_overrides", side_effect=my_overrides)
)
# Conditionally mock
stack.enter_context(patch("sys.argv", ["jrnl"] + args))
mock_stdin = stack.enter_context(
patch("sys.stdin.read", side_effect=user_input)
)
stack.enter_context(patch("sys.stdin.isatty", return_value=is_tty))
mock_input = stack.enter_context(
patch("builtins.input", side_effect=user_input)
)
mock_getpass = stack.enter_context(
patch("getpass.getpass", side_effect=password)
)
if "datetime" in mocks:
stack.enter_context(mocks["datetime"])
stack.enter_context(mocks["calendar_parse"])
stack.enter_context(
patch("jrnl.install.get_config_path", return_value=config_path)
)
stack.enter_context(
patch("jrnl.config.get_config_path", return_value=config_path)
)
mock_editor = stack.enter_context(patch("subprocess.call", side_effect=editor))
for id in factories:
mocks[id] = stack.enter_context(factories[id]())
try:
cli(args)
cli()
except StopIteration:
# This happens when input is expected, but don't have any input left
pass
except SystemExit as e:
status = e.code
cli_run["status"] = e.code
captured = capsys.readouterr()
cli_run["status"] = status
cli_run["stdout"] = captured.out
cli_run["stderr"] = captured.err
cli_run["mocks"] = {
"stdin": mock_stdin,
"input": mock_input,
"getpass": mock_getpass,
"editor": mock_editor,
}