test(coverage): close 20 audited test gaps + fix 4 bugs surfaced

framework/cli/simple_module_cli/case.py

@@ -41,6 +41,11 @@ def to_snake_case(name: str) -> str:
     s = re.sub(r"[\s\-]+", "_", name)
     s = re.sub(r"(.)([A-Z][a-z]+)", r"\1_\2", s)
     s = re.sub(r"([a-z0-9])([A-Z])", r"\1_\2", s)
+    # Collapse runs of underscores that the boundary regexes can introduce
+    # when the input already contained a separator (e.g. ``My Feature`` →
+    # ``My_Feature`` → ``My__Feature``). Without this the PyPI slug emits a
+    # double hyphen.
+    s = re.sub(r"_+", "_", s)
     return s.lower()
 
 

framework/cli/simple_module_cli/scaffolding.py

@@ -193,20 +193,30 @@ def create_module(
     template_root: Path | None = None,
 ) -> Path:
     dest = Path(dest)
+    existed_before = dest.exists()
     _require_empty_dest(dest)
     display_name = to_pascal_case(name)
     slug = to_kebab_case(name)
     package_name = to_snake_case(name)
-    _apply_template_files(
-        _resolve_template_root("module", template_root),
-        dest,
-        substitutions={
-            "{{MODULE_NAME}}": display_name,
-            "{{MODULE_SLUG}}": slug,
-            "{{PACKAGE_NAME}}": package_name,
-            "{{PACKAGE_NAME_UPPER}}": package_name.upper(),
-        },
-        path_rewrites={_PACKAGE_PATH_TOKEN: package_name},
-    )
+    try:
+        _apply_template_files(
+            _resolve_template_root("module", template_root),
+            dest,
+            substitutions={
+                "{{MODULE_NAME}}": display_name,
+                "{{MODULE_SLUG}}": slug,
+                "{{PACKAGE_NAME}}": package_name,
+                "{{PACKAGE_NAME_UPPER}}": package_name.upper(),
+            },
+            path_rewrites={_PACKAGE_PATH_TOKEN: package_name},
+        )
+    except Exception:
+        # Rollback so a half-scaffolded directory doesn't leave the user
+        # with an unparseable Python package and the impression that a
+        # retry won't work because ``dest`` is now non-empty. We only
+        # nuke the directory we created — never one we found pre-existing.
+        if not existed_before and dest.is_dir():
+            shutil.rmtree(dest, ignore_errors=True)
+        raise
     logger.info("Scaffolded module '%s' at %s (package: %s)", display_name, dest, package_name)
     return dest

framework/cli/tests/test_case.py

@@ -0,0 +1,63 @@
+"""Direct unit tests for the identifier case helpers.
+
+Every scaffolder pipes a user-supplied module name through these — a typo
+that emits ``u_r_l_path`` from ``URLPath`` would propagate into the PyPI
+slug *and* the display name. Existing tests touch ``to_pascal_case`` only
+indirectly via ``test_helpers.py``; we pin the snake/kebab forms too,
+including the acronym edge cases the docstring promises.
+"""
+
+from __future__ import annotations
+
+import pytest
+from simple_module_cli.case import to_kebab_case, to_pascal_case, to_snake_case
+
+
+class TestToSnakeCase:
+    @pytest.mark.parametrize(
+        ("raw", "expected"),
+        [
+            ("MyFeature", "my_feature"),
+            ("my-feature", "my_feature"),
+            ("my_feature", "my_feature"),
+            ("My Feature", "my_feature"),
+            ("MY_FEATURE", "my_feature"),
+            ("URLPath", "url_path"),
+            ("APIClient", "api_client"),
+            ("HTTPServer2", "http_server2"),
+            ("simple", "simple"),
+            ("simple-thing-name", "simple_thing_name"),
+            ("Already_Snake_Mixed", "already_snake_mixed"),
+            ("trailing-", "trailing_"),
+        ],
+    )
+    def test_canonicalises(self, raw, expected):
+        assert to_snake_case(raw) == expected
+
+
+class TestToKebabCase:
+    @pytest.mark.parametrize(
+        ("raw", "expected"),
+        [
+            ("MyFeature", "my-feature"),
+            ("my_feature", "my-feature"),
+            ("URLPath", "url-path"),
+        ],
+    )
+    def test_canonicalises(self, raw, expected):
+        assert to_kebab_case(raw) == expected
+
+
+class TestToPascalCase:
+    @pytest.mark.parametrize(
+        ("raw", "expected"),
+        [
+            ("my-feature", "MyFeature"),
+            ("my_feature", "MyFeature"),
+            ("MyFeature", "MyFeature"),
+            ("URLPath", "UrlPath"),  # consequence of the snake-cased pipeline
+            ("__name__", "Name"),  # empty parts dropped
+        ],
+    )
+    def test_canonicalises(self, raw, expected):
+        assert to_pascal_case(raw) == expected

framework/cli/tests/test_env_helper.py

@@ -0,0 +1,54 @@
+"""``set_env_key`` is the single helper that edits scaffold-time .env files.
+
+A regression here writes a duplicate ``KEY=`` line or, worse, leaves the old
+value unstripped — both manifest as "my recipe didn't take effect" which is
+hard to debug downstream.
+"""
+
+from __future__ import annotations
+
+from simple_module_cli._env import set_env_key
+
+
+def test_appends_to_empty_body():
+    assert set_env_key("", "FOO", "bar") == "FOO=bar\n"
+
+
+def test_replaces_existing_key():
+    body = "FOO=old\nBAR=keep\n"
+    out = set_env_key(body, "FOO", "new")
+    # Replaced line lives at the bottom (append-after-strip strategy).
+    assert "FOO=old" not in out
+    assert "FOO=new\n" in out
+    assert "BAR=keep" in out
+
+
+def test_unrelated_lines_preserved_in_order():
+    body = "A=1\nB=2\nC=3\n"
+    out = set_env_key(body, "Z", "9")
+    lines = out.splitlines()
+    assert lines[0] == "A=1"
+    assert lines[1] == "B=2"
+    assert lines[2] == "C=3"
+    assert lines[-1] == "Z=9"
+
+
+def test_idempotent_when_key_already_at_value():
+    body = "FOO=bar\n"
+    once = set_env_key(body, "FOO", "bar")
+    twice = set_env_key(once, "FOO", "bar")
+    assert once == twice == "FOO=bar\n"
+
+
+def test_prefix_match_is_exact():
+    """``KEY=`` must not match ``KEY_LONGER=``."""
+    body = "FOO_BAR=keep_me\n"
+    out = set_env_key(body, "FOO", "new")
+    assert "FOO_BAR=keep_me" in out
+    assert "FOO=new" in out
+
+
+def test_output_always_ends_with_newline():
+    body = "X=1"  # no trailing newline
+    out = set_env_key(body, "Y", "2")
+    assert out.endswith("\n")

framework/cli/tests/test_scaffold_rollback.py

@@ -0,0 +1,73 @@
+"""Scaffold rollback on partial failure.
+
+Before the rollback added to ``create_module``, a mid-pipeline error left
+the user with a non-empty destination directory — the next ``smpy new``
+invocation against the same path would then refuse to overwrite, but the
+files already written wouldn't form a valid Python package either.
+"""
+
+from __future__ import annotations
+
+from pathlib import Path
+
+import pytest
+from simple_module_cli import scaffolding
+
+
+def test_create_module_rolls_back_on_template_failure(tmp_path, monkeypatch):
+    """An exception during ``_apply_template_files`` must clear ``dest``."""
+    dest = tmp_path / "broken_module"
+
+    def boom(*_args, **_kwargs):
+        # Simulate a mid-write error after the dest directory exists but
+        # before all files have been laid down.
+        dest.mkdir(exist_ok=True)
+        (dest / "half_written.py").write_text("# truncated", encoding="utf-8")
+        raise RuntimeError("simulated template engine failure")
+
+    monkeypatch.setattr(scaffolding, "_apply_template_files", boom)
+
+    with pytest.raises(RuntimeError, match="simulated template engine failure"):
+        scaffolding.create_module(dest, "my_thing")
+
+    assert not dest.exists(), (
+        "Partial scaffold left on disk — rollback didn't fire. Subsequent "
+        "smpy new attempts at this path would refuse to overwrite."
+    )
+
+
+def test_rollback_does_not_delete_pre_existing_directory(tmp_path, monkeypatch):
+    """A pre-existing (empty) destination must stay on disk on rollback.
+
+    We can't tell from inside ``create_module`` whether ``dest`` was made
+    by us or by the caller, but the directory's *prior existence* is a
+    reliable signal: if the caller mkdir'd it, leaving their dir alone is
+    the conservative choice. (The half-written scaffold contents inside
+    are an unavoidable consequence — preventing those needs a transactional
+    file system, which we don't have.)
+    """
+    dest = tmp_path / "owned_by_caller"
+    dest.mkdir()
+
+    def boom(*_args, **_kwargs):
+        raise RuntimeError("simulated failure before any files written")
+
+    monkeypatch.setattr(scaffolding, "_apply_template_files", boom)
+
+    with pytest.raises(RuntimeError):
+        scaffolding.create_module(dest, "thing")
+
+    # The dir survives — we didn't make it.
+    assert dest.exists()
+
+
+def test_successful_scaffold_keeps_dest():
+    """Sanity check: the rollback path only fires on failure."""
+    import tempfile
+
+    with tempfile.TemporaryDirectory() as tmp:
+        dest = Path(tmp) / "real_module"
+        scaffolding.create_module(dest, "real_module")
+        assert dest.exists()
+        # The template materialises at least the package directory.
+        assert any(dest.iterdir())

framework/core/tests/test_dotenv.py

@@ -0,0 +1,127 @@
+"""Unit tests for the dependency-free ``.env`` parser.
+
+``parse_dotenv`` is invoked by the diagnostics CLI, the users-module
+bootstrap, and every worker entrypoint before settings construction — a bug
+here is hard to debug because it manifests as "the setting just isn't there".
+"""
+
+from __future__ import annotations
+
+import pytest
+from simple_module_core.dotenv import (
+    env_bool,
+    env_str,
+    load_dotenv_into_environ,
+    parse_dotenv,
+)
+
+
+class TestParseDotenv:
+    def test_missing_file_returns_empty(self, tmp_path):
+        assert parse_dotenv(tmp_path / "absent.env") == {}
+
+    def test_basic_keys(self, tmp_path):
+        env = tmp_path / ".env"
+        env.write_text("FOO=bar\nBAZ=qux\n", encoding="utf-8")
+        assert parse_dotenv(env) == {"FOO": "bar", "BAZ": "qux"}
+
+    def test_blank_lines_and_comments_ignored(self, tmp_path):
+        env = tmp_path / ".env"
+        env.write_text(
+            "# leading comment\n\nFOO=bar\n   \n# inline-style # not stripped\nBAZ=qux\n",
+            encoding="utf-8",
+        )
+        assert parse_dotenv(env) == {"FOO": "bar", "BAZ": "qux"}
+
+    def test_quotes_stripped_matching_pairs(self, tmp_path):
+        env = tmp_path / ".env"
+        env.write_text(
+            "A=\"double\"\nB='single'\nC=plain\n",
+            encoding="utf-8",
+        )
+        assert parse_dotenv(env) == {"A": "double", "B": "single", "C": "plain"}
+
+    def test_value_with_equals_keeps_remainder(self, tmp_path):
+        """KEY=foo=bar=baz must parse as KEY -> "foo=bar=baz" (first ``=`` splits).
+
+        Tokens, JWTs and database URLs frequently contain ``=`` — losing them
+        would silently break SMTP/JWT configuration in prod.
+        """
+        env = tmp_path / ".env"
+        env.write_text("URL=postgresql://u:p=raw@h/db\n", encoding="utf-8")
+        assert parse_dotenv(env) == {"URL": "postgresql://u:p=raw@h/db"}
+
+    def test_whitespace_around_key_and_value_trimmed(self, tmp_path):
+        env = tmp_path / ".env"
+        env.write_text("  KEY  =  value  \n", encoding="utf-8")
+        assert parse_dotenv(env) == {"KEY": "value"}
+
+    def test_no_equals_line_skipped(self, tmp_path):
+        env = tmp_path / ".env"
+        env.write_text("VALID=1\nbroken line without equals\nANOTHER=2\n", encoding="utf-8")
+        assert parse_dotenv(env) == {"VALID": "1", "ANOTHER": "2"}
+
+    def test_default_path_uses_sm_project_root(self, tmp_path, monkeypatch):
+        (tmp_path / ".env").write_text("ROOTED=yes\n", encoding="utf-8")
+        monkeypatch.setenv("SM_PROJECT_ROOT", str(tmp_path))
+        assert parse_dotenv() == {"ROOTED": "yes"}
+
+    def test_default_path_falls_back_to_cwd(self, tmp_path, monkeypatch):
+        (tmp_path / ".env").write_text("CWD_KEY=present\n", encoding="utf-8")
+        monkeypatch.delenv("SM_PROJECT_ROOT", raising=False)
+        monkeypatch.chdir(tmp_path)
+        assert parse_dotenv() == {"CWD_KEY": "present"}
+
+
+class TestLoadDotenvIntoEnviron:
+    def test_setdefault_semantics_preserves_existing_env(self, tmp_path, monkeypatch):
+        """Real ``os.environ`` wins over file values — same precedence as uvicorn."""
+        (tmp_path / ".env").write_text("KEY=from_file\n", encoding="utf-8")
+        monkeypatch.setenv("KEY", "from_shell")
+        load_dotenv_into_environ(tmp_path / ".env")
+        import os
+
+        assert os.environ["KEY"] == "from_shell"
+
+    def test_loads_missing_keys(self, tmp_path, monkeypatch):
+        (tmp_path / ".env").write_text("NEW_KEY_FOR_LOAD_TEST=picked_up\n", encoding="utf-8")
+        monkeypatch.delenv("NEW_KEY_FOR_LOAD_TEST", raising=False)
+        load_dotenv_into_environ(tmp_path / ".env")
+        import os
+
+        assert os.environ["NEW_KEY_FOR_LOAD_TEST"] == "picked_up"
+
+
+class TestEnvStr:
+    def test_returns_value(self, monkeypatch):
+        monkeypatch.setenv("X", "ok")
+        assert env_str("X", "default") == "ok"
+
+    def test_returns_default_when_unset(self, monkeypatch):
+        monkeypatch.delenv("X", raising=False)
+        assert env_str("X", "default") == "default"
+
+    def test_returns_default_for_whitespace_only(self, monkeypatch):
+        monkeypatch.setenv("X", "   ")
+        assert env_str("X", "default") == "default"
+
+
+class TestEnvBool:
+    @pytest.mark.parametrize("raw", ["1", "true", "TRUE", "yes", "y", "on", "  T  "])
+    def test_truthy(self, raw, monkeypatch):
+        monkeypatch.setenv("X", raw)
+        assert env_bool("X", default=False) is True
+
+    @pytest.mark.parametrize("raw", ["0", "false", "FALSE", "no", "n", "off"])
+    def test_falsy(self, raw, monkeypatch):
+        monkeypatch.setenv("X", raw)
+        assert env_bool("X", default=True) is False
+
+    def test_unset_uses_default(self, monkeypatch):
+        monkeypatch.delenv("X", raising=False)
+        assert env_bool("X", default=True) is True
+        assert env_bool("X", default=False) is False
+
+    def test_unparseable_uses_default(self, monkeypatch):
+        monkeypatch.setenv("X", "definitely-not-a-bool")
+        assert env_bool("X", default=True) is True