From 6517eb3971cdf0a5b509670a20c06f63e165c4dc Mon Sep 17 00:00:00 2001 From: Marty Oehme Date: Sat, 29 Nov 2025 10:53:10 +0100 Subject: [PATCH 1/3] ref: Reformat build_config function Make the internal function logic a litte clearer --- topen.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/topen.py b/topen.py index 611d771..a4c32a4 100755 --- a/topen.py +++ b/topen.py @@ -267,17 +267,16 @@ class TConf: def build_config() -> TConf: """Return final configuration object.""" - cli = parse_cli() + defaults = {k: opt.default for k, opt in OPTIONS.items()} env = parse_env() + cli = parse_cli() + rc_path = Path( cli.get("task_rc") or env.get("task_rc") or OPTIONS["task_rc"].default ).expanduser() + defaults["task_rc"] = rc_path # use XDG-included paths rc = parse_rc(rc_path) if rc_path.exists() else {} - # we use the 'parsed' XDG-included taskrc locations for defaults - defaults = {k: opt.default for k, opt in OPTIONS.items()} - defaults["task_rc"] = rc_path - merged = defaults | rc | env | cli # later wins return TConf.from_dict({k: v for k, v in merged.items() if v is not None}) From ff0e6cccfb8920a88f0783ff8969f96911357ad7 Mon Sep 17 00:00:00 2001 From: Marty Oehme Date: Sat, 29 Nov 2025 10:58:34 +0100 Subject: [PATCH 2/3] fix: Correctly parse boolean config options set to false Previously, when parsing config options that were set to 'false', e.g. `TOPEN_NOTES_QUIET=false` in the env vars, we would still be setting it to true. This change fixes any falsy value to be correctly parsed as False. --- test/test_configuration.py | 24 ++++++++++++++++++++++++ topen.py | 25 ++++++++++++++++++++++--- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/test/test_configuration.py b/test/test_configuration.py index 8eb6296..d6b19bb 100644 --- a/test/test_configuration.py +++ b/test/test_configuration.py @@ -63,6 +63,16 @@ class TestEnv: assert env["task_rc"] == Path("/a/dir/that/dont/exist/file") assert env["task_data"] == Path("~/somewhere/tasks") + def test_env_parses_boolean_true(self, isolate_env, monkeypatch): + monkeypatch.setenv("TOPEN_NOTES_QUIET", "true") + env = parse_env() + assert env["notes_quiet"] is True + + def test_env_parses_boolean_false(self, isolate_env, monkeypatch): + monkeypatch.setenv("TOPEN_NOTES_QUIET", "false") + env = parse_env() + assert env["notes_quiet"] is False + @pytest.fixture def fake_rc(tmp_path: Path, monkeypatch): @@ -89,6 +99,20 @@ class TestRcFile: assert rc_cfg["notes_editor"] == "micro" assert rc_cfg["notes_quiet"] is True + def test_taskrc_parses_boolean_true(self, fake_rc): + fake_rc.write_text(""" + notes.quiet=true + """) + rc_cfg = parse_rc(fake_rc) + assert rc_cfg["notes_quiet"] is True + + def test_taskrc_parses_boolean_false(self, fake_rc): + fake_rc.write_text(""" + notes.quiet=false + """) + rc_cfg = parse_rc(fake_rc) + assert rc_cfg["notes_quiet"] is False + class TestTConf: def test_paths_are_expanded(self): diff --git a/topen.py b/topen.py index a4c32a4..0143907 100755 --- a/topen.py +++ b/topen.py @@ -23,7 +23,7 @@ import subprocess import sys from dataclasses import asdict, dataclass from pathlib import Path -from typing import Any, Self, cast +from typing import Any, Callable, Self, cast from tasklib import Task, TaskWarrior @@ -130,8 +130,9 @@ class Opt: rc: str | None default: Any = None metavar: str | None = None - cast: type = str + cast: type | Callable = str help_text: str = "" + flag: bool = False def _real_path(p: Path | str) -> Path: @@ -146,6 +147,24 @@ def _determine_default_task_rc() -> Path: return _real_path("~/.config/task/taskrc") +def _strtobool(val: str) -> bool: + """Convert a string representation of truth. + + Coverts either to True or False, raising an error if it does not find a + valid value. + True values are 'y', 'yes', 't', 'true', 'on', and '1'; false values are 'n', + 'no', 'f', 'false', 'off', and '0'. + Raises ValueError if 'val' is anything else. + """ + val = val.lower() + if val in ("y", "yes", "t", "true", "on", "1"): + return True + elif val in ("n", "no", "f", "false", "off", "0"): + return False + else: + raise ValueError(f"Invalid boolean value {val}") + + OPTIONS: dict[str, Opt] = { "task_id": Opt(None, None, None, default=None), "task_rc": Opt( @@ -204,7 +223,7 @@ OPTIONS: dict[str, Opt] = { "TOPEN_NOTES_QUIET", "notes.quiet", default=False, - cast=bool, + cast=_strtobool, help_text="Silence any verbose displayed information", ), } From 13c34b08e27b13af631fbf3fb0024288669eb22d Mon Sep 17 00:00:00 2001 From: Marty Oehme Date: Sat, 29 Nov 2025 11:18:48 +0100 Subject: [PATCH 3/3] test: Restructure test files Extract the individual parsing tests (cli, env, rc) and add additional configuration test file (TConf and config builder). To extract the fixtures they have to go into an additional 'conftest.py' file for pytest to recognize and automatically import them, see: https://docs.pytest.org/en/stable/reference/fixtures.html#conftest-py-sharing-fixtures-across-multiple-files and: https://docs.pytest.org/en/stable/how-to/fixtures.html#using-fixtures-from-other-projects --- test/conftest.py | 27 ++++++ test/test_configuration.py | 178 ++++++++++++++----------------------- test/test_parse_cli.py | 48 ++++++++++ test/test_parse_env.py | 35 ++++++++ test/test_parse_rc.py | 36 ++++++++ topen.py | 14 ++- 6 files changed, 224 insertions(+), 114 deletions(-) create mode 100644 test/conftest.py create mode 100644 test/test_parse_cli.py create mode 100644 test/test_parse_env.py create mode 100644 test/test_parse_rc.py diff --git a/test/conftest.py b/test/conftest.py new file mode 100644 index 0000000..28daf73 --- /dev/null +++ b/test/conftest.py @@ -0,0 +1,27 @@ +from pathlib import Path + +import pytest + +from topen import OPTIONS + + +@pytest.fixture +def fake_id(monkeypatch): + monkeypatch.setattr("sys.argv", ["topen", "0"]) + + +@pytest.fixture +def isolate_env(monkeypatch): + # delete all existing env vars that could interfere + monkeypatch.delenv("EDITOR", raising=False) + monkeypatch.delenv("VISUAL", raising=False) + for opt in OPTIONS.values(): + if opt.env: + monkeypatch.delenv(opt.env, raising=False) + + +@pytest.fixture +def fake_rc(tmp_path: Path, monkeypatch): + rc = tmp_path / "test.taskrc" + monkeypatch.setattr(OPTIONS["task_rc"], "default", rc) + return rc diff --git a/test/test_configuration.py b/test/test_configuration.py index d6b19bb..7afe8c8 100644 --- a/test/test_configuration.py +++ b/test/test_configuration.py @@ -1,118 +1,11 @@ +# pyright: reportUnusedImport=false, reportUnusedParameter=false +# ruff: noqa: F401, F811 +# ^ Turn off for implicit pytest fixture import from pathlib import Path import pytest -from topen import OPTIONS, TConf, parse_cli, parse_env, parse_rc - - -class TestCli: - def test_cli_minimum_id(self, monkeypatch): - monkeypatch.setattr("sys.argv", ["topen", "42"]) - assert parse_cli() == {"task_id": "42"} - - def test_cli_options(self, monkeypatch): - monkeypatch.setattr( - "sys.argv", - [ - "topen", - "123", - "--extension", - "txt", - "--editor", - "vim", - "--quiet", - "True", - ], - ) - assert parse_cli() == { - "task_id": "123", - "notes_ext": "txt", - "notes_editor": "vim", - "notes_quiet": True, - } - - -@pytest.fixture -def isolate_env(monkeypatch): - # delete all existing env vars that could interfere - monkeypatch.delenv("EDITOR", raising=False) - monkeypatch.delenv("VISUAL", raising=False) - for opt in OPTIONS.values(): - if opt.env: - monkeypatch.delenv(opt.env, raising=False) - - -class TestEnv: - def test_env_notes_ext(self, isolate_env, monkeypatch): - monkeypatch.setenv("TOPEN_NOTES_DIR", "/blablubb") - monkeypatch.setenv("TOPEN_NOTES_EXT", "rst") - monkeypatch.setenv("TOPEN_NOTES_ANNOT", "qmd") - monkeypatch.setenv("TOPEN_NOTES_EDITOR", "vim") - monkeypatch.setenv("TOPEN_NOTES_QUIET", "true") - env = parse_env() - assert env["notes_dir"] == Path("/blablubb") - assert env["notes_ext"] == "rst" - assert env["notes_annot"] == "qmd" - assert env["notes_editor"] == "vim" - assert env["notes_quiet"] is True - - def test_env_task_rc(self, isolate_env, monkeypatch): - monkeypatch.setenv("TASKRC", "/a/dir/that/dont/exist/file") - monkeypatch.setenv("TASKDATA", "~/somewhere/tasks") - env = parse_env() - assert env["task_rc"] == Path("/a/dir/that/dont/exist/file") - assert env["task_data"] == Path("~/somewhere/tasks") - - def test_env_parses_boolean_true(self, isolate_env, monkeypatch): - monkeypatch.setenv("TOPEN_NOTES_QUIET", "true") - env = parse_env() - assert env["notes_quiet"] is True - - def test_env_parses_boolean_false(self, isolate_env, monkeypatch): - monkeypatch.setenv("TOPEN_NOTES_QUIET", "false") - env = parse_env() - assert env["notes_quiet"] is False - - -@pytest.fixture -def fake_rc(tmp_path: Path, monkeypatch): - rc = tmp_path / "test.taskrc" - monkeypatch.setattr(OPTIONS["task_rc"], "default", rc) - return rc - - -class TestRcFile: - def test_taskrc_parsing(self, fake_rc): - fake_rc.write_text(""" - data.location=~/.taskies - notes.dir=/there - notes.ext=yaml - notes.annot=Boo! - notes.editor=micro - notes.quiet=true - """) - rc_cfg = parse_rc(fake_rc) - assert rc_cfg["task_data"] == Path("~/.taskies") - assert rc_cfg["notes_dir"] == Path("/there") - assert rc_cfg["notes_ext"] == "yaml" - assert rc_cfg["notes_annot"] == "Boo!" - assert rc_cfg["notes_editor"] == "micro" - assert rc_cfg["notes_quiet"] is True - - def test_taskrc_parses_boolean_true(self, fake_rc): - fake_rc.write_text(""" - notes.quiet=true - """) - rc_cfg = parse_rc(fake_rc) - assert rc_cfg["notes_quiet"] is True - - def test_taskrc_parses_boolean_false(self, fake_rc): - fake_rc.write_text(""" - notes.quiet=false - """) - rc_cfg = parse_rc(fake_rc) - assert rc_cfg["notes_quiet"] is False - +from topen import TConf, build_config class TestTConf: def test_paths_are_expanded(self): @@ -140,7 +33,68 @@ class TestTConf: ({"VISUAL": "nvim", "EDITOR": "notepad"}, "notepad"), ], ) - def test_editor_env_resolution(isolate_env, monkeypatch, env, expected): + def test_editor_env_resolution(self, isolate_env, monkeypatch, env, expected): for k, v in env.items(): monkeypatch.setenv(k, v) assert TConf(0).notes_editor == expected + + +class TestBuildConfigPrecedence: + """ + All tests exercise the same key (notes_ext) to keep the assertions short. + Each source sets a different value so we can be sure the right one wins. + """ + + def test_defaults_only(self, fake_rc, monkeypatch, isolate_env, fake_id): + cfg = build_config() + assert cfg.notes_ext == "md" + + def test_taskrc_overrides_defaults( + self, fake_rc, monkeypatch, isolate_env, fake_id + ): + fake_rc.write_text("notes.ext=from-rc\n") + cfg = build_config() + assert cfg.notes_ext == "from-rc" + + def test_env_overrides_taskrc(self, fake_rc, monkeypatch, isolate_env, fake_id): + fake_rc.write_text("notes.ext=from-rc\n") + monkeypatch.setenv("TOPEN_NOTES_EXT", "from-env") + cfg = build_config() + assert cfg.notes_ext == "from-env" + + def test_cli_overrides_env(self, fake_rc, monkeypatch, isolate_env): + fake_rc.write_text("notes.ext=from-rc\n") + monkeypatch.setenv("TOPEN_NOTES_EXT", "from-env") + monkeypatch.setattr("sys.argv", ["topen", "0", "--extension", "from-cli"]) + cfg = build_config() + assert cfg.notes_ext == "from-cli" + + def test_cli_overrides_everything(self, fake_rc, monkeypatch, isolate_env): + fake_rc.write_text("notes.ext=from-rc\nnotes.dir=/rc-dir\nnotes.editor=joe") + monkeypatch.setenv("TOPEN_NOTES_EXT", "from-env") + monkeypatch.setenv("TOPEN_NOTES_DIR", "/env-dir") + monkeypatch.setenv("EDITOR", "emacs") + # CLI wins + monkeypatch.setattr( + "sys.argv", + [ + "topen", + "0", + "--extension", + "cli-ext", + "--notes-dir", + "cli-dir", + "--editor", + "helix", + ], + ) + cfg = build_config() + assert cfg.notes_ext == "cli-ext" + assert cfg.notes_dir == Path("cli-dir") + assert cfg.notes_editor == "helix" + + # sanity check that the task-id coming from CLI is preserved + def test_cli_supplies_task_id(self, fake_rc, monkeypatch, isolate_env): + monkeypatch.setattr("sys.argv", ["topen", "42"]) + cfg = build_config() + assert cfg.task_id == "42" diff --git a/test/test_parse_cli.py b/test/test_parse_cli.py new file mode 100644 index 0000000..984dcf5 --- /dev/null +++ b/test/test_parse_cli.py @@ -0,0 +1,48 @@ +from pathlib import Path + +from topen import parse_cli + + +class TestCli: + def test_cli_minimum_id(self, monkeypatch): + monkeypatch.setattr("sys.argv", ["topen", "42"]) + assert parse_cli() == {"task_id": "42"} + + def test_cli_options(self, monkeypatch): + monkeypatch.setattr( + "sys.argv", + [ + "topen", + "123", + "--extension", + "txt", + "--editor", + "vim", + "--annotation", + "HERENOTE", + ], + ) + assert parse_cli() == { + "task_id": "123", + "notes_ext": "txt", + "notes_editor": "vim", + "notes_annot": "HERENOTE", + } + + def test_cli_notes_quiet_is_flag(self, monkeypatch): + monkeypatch.setattr( + "sys.argv", + [ + "topen", + "123", + "--quiet", + ], + ) + assert parse_cli()["notes_quiet"] is True + + def test_cli_parses_paths(self, monkeypatch): + monkeypatch.setattr( + "sys.argv", + ["topen", "123", "--notes-dir", "/somewhere/else"], + ) + assert parse_cli()["notes_dir"] == Path("/somewhere/else") diff --git a/test/test_parse_env.py b/test/test_parse_env.py new file mode 100644 index 0000000..2f637e0 --- /dev/null +++ b/test/test_parse_env.py @@ -0,0 +1,35 @@ +from pathlib import Path + +from topen import parse_env + + +class TestEnv: + def test_env_notes_ext(self, isolate_env, monkeypatch): + monkeypatch.setenv("TOPEN_NOTES_DIR", "/blablubb") + monkeypatch.setenv("TOPEN_NOTES_EXT", "rst") + monkeypatch.setenv("TOPEN_NOTES_ANNOT", "qmd") + monkeypatch.setenv("TOPEN_NOTES_EDITOR", "vim") + monkeypatch.setenv("TOPEN_NOTES_QUIET", "true") + env = parse_env() + assert env["notes_dir"] == Path("/blablubb") + assert env["notes_ext"] == "rst" + assert env["notes_annot"] == "qmd" + assert env["notes_editor"] == "vim" + assert env["notes_quiet"] is True + + def test_env_task_rc(self, isolate_env, monkeypatch): + monkeypatch.setenv("TASKRC", "/a/dir/that/dont/exist/file") + monkeypatch.setenv("TASKDATA", "~/somewhere/tasks") + env = parse_env() + assert env["task_rc"] == Path("/a/dir/that/dont/exist/file") + assert env["task_data"] == Path("~/somewhere/tasks") + + def test_env_parses_boolean_true(self, isolate_env, monkeypatch): + monkeypatch.setenv("TOPEN_NOTES_QUIET", "true") + env = parse_env() + assert env["notes_quiet"] is True + + def test_env_parses_boolean_false(self, isolate_env, monkeypatch): + monkeypatch.setenv("TOPEN_NOTES_QUIET", "false") + env = parse_env() + assert env["notes_quiet"] is False diff --git a/test/test_parse_rc.py b/test/test_parse_rc.py new file mode 100644 index 0000000..c46b094 --- /dev/null +++ b/test/test_parse_rc.py @@ -0,0 +1,36 @@ +from pathlib import Path + +from topen import parse_rc + + +class TestRcFile: + def test_taskrc_parsing(self, fake_rc): + fake_rc.write_text(""" + data.location=~/.taskies + notes.dir=/there + notes.ext=yaml + notes.annot=Boo! + notes.editor=micro + notes.quiet=true + """) + rc_cfg = parse_rc(fake_rc) + assert rc_cfg["task_data"] == Path("~/.taskies") + assert rc_cfg["notes_dir"] == Path("/there") + assert rc_cfg["notes_ext"] == "yaml" + assert rc_cfg["notes_annot"] == "Boo!" + assert rc_cfg["notes_editor"] == "micro" + assert rc_cfg["notes_quiet"] is True + + def test_taskrc_parses_boolean_true(self, fake_rc): + fake_rc.write_text(""" + notes.quiet=true + """) + rc_cfg = parse_rc(fake_rc) + assert rc_cfg["notes_quiet"] is True + + def test_taskrc_parses_boolean_false(self, fake_rc): + fake_rc.write_text(""" + notes.quiet=false + """) + rc_cfg = parse_rc(fake_rc) + assert rc_cfg["notes_quiet"] is False diff --git a/topen.py b/topen.py index 0143907..e04e39e 100755 --- a/topen.py +++ b/topen.py @@ -132,7 +132,7 @@ class Opt: metavar: str | None = None cast: type | Callable = str help_text: str = "" - flag: bool = False + is_flag: bool = False def _real_path(p: Path | str) -> Path: @@ -224,7 +224,8 @@ OPTIONS: dict[str, Opt] = { "notes.quiet", default=False, cast=_strtobool, - help_text="Silence any verbose displayed information", + help_text="Silence any verbosely displayed information", + is_flag=True, ), } @@ -321,6 +322,15 @@ you view the task. for key, opt in OPTIONS.items(): if opt.cli is None: continue + if opt.is_flag: + parser.add_argument( + *opt.cli, + dest=key, + help=opt.help_text, + default=None, + action="store_true", + ) + continue parser.add_argument( *opt.cli, dest=key,