From ea6df4705cf3256f012fe6be036b2ccae6623a18 Mon Sep 17 00:00:00 2001 From: Kevin Date: Sat, 21 May 2022 14:06:07 -0700 Subject: [PATCH] Always expand all paths (journals, templates, etc) (#1468) * Refactored path expansion calls into a new path.py file This also fixed bugs with relative journal and template paths. * Update tests for new path functions Also, make the tests specific to Windows, Mac & Linux Co-authored-by: Jonathan Wren Co-authored-by: Micah Jerome Ellison --- jrnl/Journal.py | 3 +- jrnl/config.py | 9 ++-- jrnl/install.py | 13 ++--- jrnl/jrnl.py | 7 ++- jrnl/path.py | 13 +++++ jrnl/upgrade.py | 11 ++--- pyproject.toml | 4 ++ tests/conftest.py | 30 ++++++++++++ tests/unit/test_path.py | 103 ++++++++++++++++++++++++++++++++++++++++ 9 files changed, 172 insertions(+), 21 deletions(-) create mode 100644 jrnl/path.py create mode 100644 tests/unit/test_path.py diff --git a/jrnl/Journal.py b/jrnl/Journal.py index 13c5363a..987c5426 100644 --- a/jrnl/Journal.py +++ b/jrnl/Journal.py @@ -11,6 +11,7 @@ import sys from . import Entry from . import time from .prompt import yesno +from .path import expand_path class Tag: @@ -410,7 +411,7 @@ def open_journal(journal_name, config, legacy=False): backwards compatibility with jrnl 1.x """ config = config.copy() - config["journal"] = os.path.expanduser(os.path.expandvars(config["journal"])) + config["journal"] = expand_path(config["journal"]) if os.path.isdir(config["journal"]): if config["encrypt"]: diff --git a/jrnl/config.py b/jrnl/config.py index 2b07b14b..67284c4a 100644 --- a/jrnl/config.py +++ b/jrnl/config.py @@ -15,6 +15,7 @@ from jrnl.messages import MsgType from .color import ERROR_COLOR from .color import RESET_COLOR from .output import list_journals +from .path import home_dir # Constants DEFAULT_CONFIG_NAME = "jrnl.yaml" @@ -83,9 +84,7 @@ def get_config_path(): ), ) - return os.path.join( - config_directory_path or os.path.expanduser("~"), DEFAULT_CONFIG_NAME - ) + return os.path.join(config_directory_path or home_dir(), DEFAULT_CONFIG_NAME) def get_default_config(): @@ -112,9 +111,7 @@ def get_default_config(): def get_default_journal_path(): - journal_data_path = xdg.BaseDirectory.save_data_path( - XDG_RESOURCE - ) or os.path.expanduser("~") + journal_data_path = xdg.BaseDirectory.save_data_path(XDG_RESOURCE) or home_dir() return os.path.join(journal_data_path, DEFAULT_JOURNAL_NAME) diff --git a/jrnl/install.py b/jrnl/install.py index f8322fba..0e29eb48 100644 --- a/jrnl/install.py +++ b/jrnl/install.py @@ -7,6 +7,9 @@ import logging import os import sys +from .path import home_dir +from .path import absolute_path +from .path import expand_path from .config import DEFAULT_JOURNAL_KEY from .config import get_config_path from .config import get_default_config @@ -45,7 +48,7 @@ def find_default_config(): config_path = ( get_config_path() if os.path.exists(get_config_path()) - else os.path.join(os.path.expanduser("~"), ".jrnl_config") + else os.path.join(home_dir(), ".jrnl_config") ) return config_path @@ -101,11 +104,9 @@ def install(): # Where to create the journal? default_journal_path = get_default_journal_path() path_query = f"Path to your journal file (leave blank for {default_journal_path}): " - journal_path = os.path.abspath(input(path_query).strip() or default_journal_path) + journal_path = absolute_path(input(path_query).strip() or default_journal_path) default_config = get_default_config() - default_config["journals"][DEFAULT_JOURNAL_KEY] = os.path.expanduser( - os.path.expandvars(journal_path) - ) + default_config["journals"][DEFAULT_JOURNAL_KEY] = journal_path # If the folder doesn't exist, create it path = os.path.split(default_config["journals"][DEFAULT_JOURNAL_KEY])[0] @@ -138,7 +139,7 @@ def _initialize_autocomplete(): def _autocomplete_path(text, state): - expansions = glob.glob(os.path.expanduser(os.path.expandvars(text)) + "*") + expansions = glob.glob(expand_path(text) + "*") expansions = [e + "/" if os.path.isdir(e) else e for e in expansions] expansions.append(None) return expansions[state] diff --git a/jrnl/jrnl.py b/jrnl/jrnl.py index 7c09734e..aa26d1e1 100644 --- a/jrnl/jrnl.py +++ b/jrnl/jrnl.py @@ -14,6 +14,7 @@ from .editor import get_text_from_editor from .editor import get_text_from_stdin from . import time from .override import apply_overrides +from .path import expand_path from jrnl.exception import JrnlException from jrnl.messages import Message @@ -217,8 +218,10 @@ def _get_editor_template(config, **kwargs): logging.debug("Write mode: no template configured") return "" + template_path = expand_path(config["template"]) + try: - template = open(config["template"]).read() + template = open(template_path).read() logging.debug("Write mode: template loaded: %s", template) except OSError: logging.error("Write mode: template not loaded") @@ -226,7 +229,7 @@ def _get_editor_template(config, **kwargs): Message( MsgText.CantReadTemplate, MsgType.ERROR, - {"template": config["template"]}, + {"template": template_path}, ) ) diff --git a/jrnl/path.py b/jrnl/path.py new file mode 100644 index 00000000..bb465d6e --- /dev/null +++ b/jrnl/path.py @@ -0,0 +1,13 @@ +import os.path + + +def home_dir(): + return os.path.expanduser("~") + + +def expand_path(path): + return os.path.expanduser(os.path.expandvars(path)) + + +def absolute_path(path): + return os.path.abspath(expand_path(path)) diff --git a/jrnl/upgrade.py b/jrnl/upgrade.py index 3027f4e7..b9fc0092 100644 --- a/jrnl/upgrade.py +++ b/jrnl/upgrade.py @@ -11,6 +11,7 @@ from .config import is_config_json from .config import load_config from .config import scope_config from .prompt import yesno +from .path import expand_path from jrnl.output import print_msg @@ -22,7 +23,7 @@ from jrnl.messages import MsgType def backup(filename, binary=False): print(f" Created a backup at {filename}.backup", file=sys.stderr) - filename = os.path.expanduser(os.path.expandvars(filename)) + filename = expand_path(filename) try: with open(filename, "rb" if binary else "r") as original: @@ -72,15 +73,13 @@ older versions of jrnl anymore. for journal_name, journal_conf in config["journals"].items(): if isinstance(journal_conf, dict): - path = journal_conf.get("journal") + path = expand_path(journal_conf.get("journal")) encrypt = journal_conf.get("encrypt") else: encrypt = config.get("encrypt") - path = journal_conf + path = expand_path(journal_conf) - if os.path.exists(os.path.expanduser(path)): - path = os.path.expanduser(path) - else: + if not os.path.exists(path): print(f"\nError: {path} does not exist.") continue diff --git a/pyproject.toml b/pyproject.toml index 88861740..4885344e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -81,6 +81,10 @@ required_plugins = [ ] markers = [ "todo", + "skip_win", + "skip_posix", + "on_win", + "on_posix", ] addopts = [ "--pdbcls=IPython.terminal.debugger:Pdb" diff --git a/tests/conftest.py b/tests/conftest.py index fbbc3068..c4dc5ef5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,8 +2,10 @@ # License: https://www.gnu.org/licenses/gpl-3.0.html from pytest import mark +from pytest import skip from jrnl.os_compat import on_windows +from jrnl.os_compat import on_posix pytest_plugins = [ @@ -15,11 +17,39 @@ pytest_plugins = [ def pytest_bdd_apply_tag(tag, function): + # skip markers if tag == "skip_win": marker = mark.skipif(on_windows(), reason="Skip test on Windows") + elif tag == "skip_posix": + marker = mark.skipif(on_posix(), reason="Skip test on Mac/Linux") + + # only on OS markers + elif tag == "on_win": + marker = mark.skipif(not on_windows(), reason="Skip test not on Windows") + elif tag == "on_posix": + marker = mark.skipif(not on_posix(), reason="Skip test not on Mac/Linux") else: # Fall back to pytest-bdd's default behavior return None marker(function) return True + + +def pytest_runtest_setup(item): + markers = [mark.name for mark in item.iter_markers()] + + on_win = on_windows() + on_nix = on_posix() + + if "skip_win" in markers and on_win: + skip("Skip test on Windows") + + if "skip_posix" in markers and on_nix: + skip("Skip test on Mac/Linux") + + if "on_win" in markers and not on_win: + skip("Skip test not on Windows") + + if "on_posix" in markers and not on_nix: + skip("Skip test not on Mac/Linux") diff --git a/tests/unit/test_path.py b/tests/unit/test_path.py new file mode 100644 index 00000000..a13b89e5 --- /dev/null +++ b/tests/unit/test_path.py @@ -0,0 +1,103 @@ +import pytest +import random +import string + +from os import getenv +from unittest.mock import patch + +from jrnl.path import home_dir +from jrnl.path import expand_path +from jrnl.path import absolute_path + + +@pytest.fixture +def home_dir_str(monkeypatch): + username = "username" + monkeypatch.setenv("USERPROFILE", username) # for windows + monkeypatch.setenv("HOME", username) # for *nix + return username + + +@pytest.fixture +def random_test_var(monkeypatch): + name = f"JRNL_TEST_{''.join(random.sample(string.ascii_uppercase, 10))}" + val = "".join(random.sample(string.ascii_lowercase, 25)) + monkeypatch.setenv(name, val) + return (name, val) + + +def test_home_dir(home_dir_str): + assert home_dir() == home_dir_str + + +@pytest.mark.on_posix +@pytest.mark.parametrize( + "path", + ["~"], +) +def test_expand_path_actually_expands_mac_linux(path): + # makes sure that path isn't being returns as-is + assert expand_path(path) != path + + +@pytest.mark.on_win +@pytest.mark.parametrize( + "path", + ["~", "%USERPROFILE%"], +) +def test_expand_path_actually_expands_windows(path): + # makes sure that path isn't being returns as-is + assert expand_path(path) != path + + +@pytest.mark.on_posix +@pytest.mark.parametrize( + "paths", + [ + ["~", "HOME"], + ], +) +def test_expand_path_expands_into_correct_value_mac_linux(paths): + input_path, expected_path = paths[0], paths[1] + assert expand_path(input_path) == getenv(expected_path) + + +@pytest.mark.on_win +@pytest.mark.parametrize( + "paths", + [ + ["~", "USERPROFILE"], + ["%USERPROFILE%", "USERPROFILE"], + ], +) +def test_expand_path_expands_into_correct_value_windows(paths): + input_path, expected_path = paths[0], paths[1] + assert expand_path(input_path) == getenv(expected_path) + + +@pytest.mark.on_posix +@pytest.mark.parametrize("_", range(25)) +def test_expand_path_expands_into_random_env_value_mac_linux(_, random_test_var): + var_name, var_value = random_test_var[0], random_test_var[1] + assert expand_path(var_name) == var_name + assert expand_path(f"${var_name}") == var_value # mac & linux + assert expand_path(f"${var_name}") == getenv(var_name) + + +@pytest.mark.on_win +@pytest.mark.parametrize("_", range(25)) +def test_expand_path_expands_into_random_env_value_windows(_, random_test_var): + var_name, var_value = random_test_var[0], random_test_var[1] + assert expand_path(var_name) == var_name + assert expand_path(f"%{var_name}%") == var_value # windows + assert expand_path(f"%{var_name}%") == getenv(var_name) + + +@patch("jrnl.path.expand_path") +@patch("os.path.abspath") +def test_absolute_path(mock_abspath, mock_expand_path): + test_val = "test_value" + + assert absolute_path(test_val) == mock_abspath.return_value + mock_expand_path.assert_called_with(test_val) + mock_abspath.assert_called_with(mock_expand_path.return_value)