cli.py's typer._click.core.format_help patch was applied a second time outside the try/except that exists to let a future typer without typer._click degrade to plain Click help instead of crashing on import. That second, unconditional line referenced two names the failed import never defines, so the exact case the fallback exists for raised NameError on every invocation instead. Removed; a subprocess test with a targeted import hook (not sys.modules poisoning, which breaks typer itself, and not importlib.reload, which would stay green against the bug) pins that the fallback now actually falls back. The new command's cli_contract record promised kb/concepts/<Name>.md and kb/sources/Source - <Name>.md - both types have since grown a layout: that files a page under a subtype-computed subdirectory, the same rule an entity or a project already follows. Notes for concept and source now name the subdirectory and where it comes from; comparison was checked against its type-spec and left alone. Also found and fixed: new source's example line omitted source_type, fidelity and authority, so it could never succeed as written. A new test reads each variant's note out of the record and checks the written path against the pattern it promises, instead of against a path re-typed into the test - which is what let the existing per-type path tests stay green through this exact drift. tools/CONTRACT.md regenerated via `wikitool docs contract --apply`; pytest (1550), docs verify and instructions verify all green. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SnAJ7Z3CpVD3PRbN73QtU2
This commit is contained in:
1 parent
b8ed8bd610
commit
6c0ebcc4f0
7 files changed
+240
-13
No files matched your search
+3
-3
@@ -155,9 +155,9 @@ Scaffold a new wiki page of any type.
|
||||
**SYNOPSIS**
|
||||
|
||||
- `wikitool new <type-name> --name "<Name>" [--type <path>] [--set field=value ...]` - Any type; its type-spec decides fields, directory, title prefix and template.
|
||||
- `wikitool new entity --name "<Name>" --set entity_type=<t> [--set tags=a,b] [--set related=X,Y] [--set sources="Source - Z"] [--set provenance=sourced|general|mixed]` - Writes `kb/entities/<subdir>/<Name>.md`
|
||||
- `wikitool new concept --name "<Name>" --set concept_type=<t> ...` - Writes `kb/concepts/<Name>.md`
|
||||
- `wikitool new source --name "<Name>" --set raw_files=raw/notes/x.md,raw/notes/y.md [--set source_url=<URL>] [--set entities=A,B] [--set concepts=C,D]` - Writes `kb/sources/Source - <Name>.md` (prefix added automatically) with a `raw_files:` list; rejects paths that don't exist
|
||||
- `wikitool new entity --name "<Name>" --set entity_type=<t> [--set tags=a,b] [--set related=X,Y] [--set sources="Source - Z"] [--set provenance=sourced|general|mixed]` - Writes `kb/entities/<subdir>/<Name>.md` - `<subdir>` from `entity_type` via the type-spec's `layout:`
|
||||
- `wikitool new concept --name "<Name>" --set concept_type=<t> ...` - Writes `kb/concepts/<subdir>/<Name>.md` - `<subdir>` from `concept_type` via the type-spec's `layout:`
|
||||
- `wikitool new source --name "<Name>" --set source_type=<t> --set raw_files=raw/notes/x.md,raw/notes/y.md --set fidelity=<f> --set authority=<a> [--set source_url=<URL>] [--set entities=A,B] [--set concepts=C,D]` - Writes `kb/sources/<subdir>/Source - <Name>.md` (prefix added automatically; `<subdir>` from `source_type` via the type-spec's `layout:`) with a `raw_files:` list; rejects paths that don't exist
|
||||
- `wikitool new comparison --name "X vs Y" --set entities=X,Y` - Writes `kb/comparisons/X vs Y.md`
|
||||
- `wikitool new project --name "<Name>" --set responsibility=<bereich> [--resume]` - Writes `kb/gtd/<bereich>/<Name>.md` and, with a task tracker configured, a same-named tracker project
|
||||
|
||||
|
||||
@@ -262,9 +262,6 @@ except (ImportError, AttributeError):
|
||||
pass
|
||||
|
||||
|
||||
_typer_click_core.Command.format_help = _contract_format_help
|
||||
|
||||
|
||||
def main() -> None:
|
||||
# Iteration Budget Gate / Loop-Breaker (see instructions/gates.md
|
||||
# "Iteration Budget Gate and loop-breaker"): recorded and enforced here,
|
||||
|
||||
@@ -337,17 +337,21 @@ def _ensure_tracker_project(page_title: str, *, resume: bool) -> Optional[str]:
|
||||
usage='new entity --name "<Name>" --set entity_type=<t> [--set tags=a,b] '
|
||||
"[--set related=X,Y] [--set sources=\"Source - Z\"] "
|
||||
"[--set provenance=sourced|general|mixed]",
|
||||
notes="Writes `kb/entities/<subdir>/<Name>.md`",
|
||||
notes="Writes `kb/entities/<subdir>/<Name>.md` - `<subdir>` from `entity_type` "
|
||||
"via the type-spec's `layout:`",
|
||||
),
|
||||
cli_contract.Variant(
|
||||
usage='new concept --name "<Name>" --set concept_type=<t> ...',
|
||||
notes="Writes `kb/concepts/<Name>.md`",
|
||||
notes="Writes `kb/concepts/<subdir>/<Name>.md` - `<subdir>` from `concept_type` "
|
||||
"via the type-spec's `layout:`",
|
||||
),
|
||||
cli_contract.Variant(
|
||||
usage='new source --name "<Name>" --set raw_files=raw/notes/x.md,raw/notes/y.md '
|
||||
usage='new source --name "<Name>" --set source_type=<t> '
|
||||
"--set raw_files=raw/notes/x.md,raw/notes/y.md --set fidelity=<f> --set authority=<a> "
|
||||
"[--set source_url=<URL>] [--set entities=A,B] [--set concepts=C,D]",
|
||||
notes="Writes `kb/sources/Source - <Name>.md` (prefix added automatically) with a "
|
||||
"`raw_files:` list; rejects paths that don't exist",
|
||||
notes="Writes `kb/sources/<subdir>/Source - <Name>.md` (prefix added automatically; "
|
||||
"`<subdir>` from `source_type` via the type-spec's `layout:`) with a `raw_files:` "
|
||||
"list; rejects paths that don't exist",
|
||||
),
|
||||
cli_contract.Variant(
|
||||
usage='new comparison --name "X vs Y" --set entities=X,Y',
|
||||
|
||||
@@ -266,6 +266,55 @@ def test_a_usage_error_is_unframed_and_names_wikitool(monkeypatch):
|
||||
assert not FRAME_CHARS & set(text)
|
||||
|
||||
|
||||
# --- the `typer._click` fallback actually falls back (Gitea #148) ---
|
||||
#
|
||||
# `cli.py`'s help patch is wrapped in `try/except (ImportError, AttributeError)`
|
||||
# specifically so a future typer without `typer._click.core` degrades to plain
|
||||
# Click help instead of crashing every invocation. A subprocess with a targeted
|
||||
# import hook is what actually exercises that: `sys.modules[...] = None` was
|
||||
# tried and rejected, because typer itself lazily imports `typer._click.decorators`
|
||||
# from inside `get_help_option` - poisoning the module that way breaks typer's own
|
||||
# `-h` handling regardless of what `cli.py` does, on both the buggy and the fixed
|
||||
# code. `importlib.reload(chemenu.cli)` was tried and rejected too: reload runs
|
||||
# in the same module `__dict__`, so `_typer_click_core`/`_contract_format_help`
|
||||
# from the first (real) import survive and the broken module-level line at the
|
||||
# end runs successfully - a test built that way would stay green against the bug
|
||||
# it exists to catch.
|
||||
_IMPORT_HOOK = """
|
||||
import builtins, runpy, sys
|
||||
_real_import = builtins.__import__
|
||||
def _guarded(name, globals=None, locals=None, fromlist=(), level=0):
|
||||
if name == "typer._click.core" and (globals or {}).get("__name__") in ("chemenu.cli", "__main__"):
|
||||
raise ImportError("simulated: typer without _click, for Gitea #148")
|
||||
return _real_import(name, globals, locals, fromlist, level)
|
||||
builtins.__import__ = _guarded
|
||||
sys.argv = ["wikitool", "search", "-h"]
|
||||
runpy.run_module("chemenu.cli", run_name="__main__")
|
||||
"""
|
||||
|
||||
|
||||
def test_missing_typer_click_falls_back_to_plain_click_help_instead_of_crashing(tmp_path):
|
||||
tools_dir = Path(__file__).resolve().parents[2]
|
||||
env = dict(os.environ)
|
||||
env["CHEMENU_ROOT"] = str(tmp_path)
|
||||
env["WIKI_TRACE_DIR"] = str(tmp_path / "trace")
|
||||
env["WIKITOOL_SESSION_ID"] = "test-148-typer-click-fallback"
|
||||
result = subprocess.run(
|
||||
[sys.executable, "-c", _IMPORT_HOOK],
|
||||
cwd=str(tools_dir),
|
||||
env=env,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=30,
|
||||
)
|
||||
assert result.returncode == 0, result.stdout + result.stderr
|
||||
assert "Traceback" not in result.stderr
|
||||
assert result.stdout.startswith("Usage:")
|
||||
# Click's own plain help, not the cli_contract-rendered record.
|
||||
assert "SYNOPSIS" not in result.stdout
|
||||
assert "EXIT STATUS" not in result.stdout
|
||||
|
||||
|
||||
def test_every_record_shows_at_least_one_example():
|
||||
"""Every real command record carries a copyable example (Gitea #142)."""
|
||||
missing = [path for path, rec in cli_contract.all_records().items() if not rec.examples]
|
||||
|
||||
@@ -1,9 +1,13 @@
|
||||
import contextlib
|
||||
import http.server
|
||||
import json
|
||||
import re
|
||||
import threading
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from chemenu import cli_contract
|
||||
from chemenu.commands._util import coerce_set_value, parse_set_fields
|
||||
from chemenu.commands.new_page import _page_subdir
|
||||
from chemenu.frontmatter_io import read_page
|
||||
@@ -690,6 +694,132 @@ def test_new_comparison_rejects_single_entity(monkeypatch, kb_dir):
|
||||
assert result.exit_code != 0
|
||||
|
||||
|
||||
# --- the `new` record's own path notes match what `new` actually writes
|
||||
# (Gitea #150) ---
|
||||
#
|
||||
# Coupled to the record's notes text, not to a path hand-copied into the
|
||||
# test - the existing per-type tests above (e.g.
|
||||
# `test_new_concept_creates_page_in_its_subtype_area`) assert a path against
|
||||
# the *code*, so a note that drifted out of sync with the code would still
|
||||
# pass every one of them. This reads the note itself and turns it into the
|
||||
# pattern it promises, so a drift like #150's (the `concept`/`source` notes
|
||||
# lost their `<subdir>` when `layout:` was added to their type-specs) fails
|
||||
# here even if every code-level path test stays green.
|
||||
|
||||
def _path_pattern_from_note(notes: str, name: str) -> re.Pattern:
|
||||
"""The first backtick-quoted span in a `new` variant's notes is its path
|
||||
template. `<Name>` becomes the literal name under test; any other
|
||||
`<...>` placeholder (a type-spec-computed subdirectory) becomes a
|
||||
wildcard segment. A note with no placeholder at all (`comparison`, which
|
||||
genuinely has none) is matched as the literal path it names. The leading
|
||||
`kb/` is stripped: the actual path under test is already relative to
|
||||
`kb_dir`, which *is* that root."""
|
||||
template = re.search(r"`([^`]+)`", notes).group(1)
|
||||
assert template.startswith("kb/"), template
|
||||
template = template[len("kb/"):]
|
||||
pattern = "".join(
|
||||
re.escape(name) if part == "<Name>"
|
||||
else r"[^/]+" if re.fullmatch(r"<[^>]+>", part)
|
||||
else re.escape(part)
|
||||
for part in re.split(r"(<[^>]+>)", template)
|
||||
)
|
||||
return re.compile(pattern + r"\Z")
|
||||
|
||||
|
||||
_NEW_PATH_VARIANTS = [
|
||||
v for v in cli_contract.get("new").synopsis if v.notes.startswith("Writes `kb/")
|
||||
]
|
||||
|
||||
|
||||
def _new_variant_notes(usage_prefix: str) -> str:
|
||||
for variant in _NEW_PATH_VARIANTS:
|
||||
if variant.usage.startswith(usage_prefix):
|
||||
return variant.notes
|
||||
raise AssertionError(f"no `new` record variant starts with {usage_prefix!r}")
|
||||
|
||||
|
||||
def test_every_writes_kb_variant_has_a_path_case():
|
||||
"""A future variant whose notes promise a `kb/` path but that nobody
|
||||
parametrized below would otherwise go unchecked - same failure shape as
|
||||
#150 itself, one level up."""
|
||||
covered_prefixes = ("new entity ", "new concept ", "new source ", "new comparison ", "new project ")
|
||||
uncovered = [
|
||||
v.usage for v in _NEW_PATH_VARIANTS
|
||||
if not any(v.usage.startswith(p) for p in covered_prefixes)
|
||||
]
|
||||
assert uncovered == []
|
||||
|
||||
|
||||
def test_new_entity_path_matches_its_record_note(monkeypatch, kb_dir):
|
||||
name = "Widget"
|
||||
result = _invoke_new(monkeypatch, kb_dir, [
|
||||
"new", "entity", "--name", name, "--set", "entity_type=tool",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
pattern = _path_pattern_from_note(_new_variant_notes("new entity "), name)
|
||||
written = next(kb_dir.glob(f"entities/**/{name}.md"))
|
||||
assert pattern.match(str(written.relative_to(kb_dir)))
|
||||
|
||||
|
||||
def test_new_concept_path_matches_its_record_note(monkeypatch, kb_dir):
|
||||
name = "Event Sourcing"
|
||||
result = _invoke_new(monkeypatch, kb_dir, [
|
||||
"new", "concept", "--name", name, "--set", "concept_type=pattern",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
pattern = _path_pattern_from_note(_new_variant_notes("new concept "), name)
|
||||
written = next(kb_dir.glob(f"concepts/**/{name}.md"))
|
||||
assert pattern.match(str(written.relative_to(kb_dir)))
|
||||
|
||||
|
||||
def test_new_source_path_matches_its_record_note_and_its_usage_is_runnable(monkeypatch, kb_dir):
|
||||
"""Also #150's second finding: the `usage` line itself omitted
|
||||
`source_type`/`fidelity`/`authority` and could never succeed as written.
|
||||
Running the corrected `usage` verbatim (substituting `<Name>`/`<t>`/etc.)
|
||||
is the test that it now can."""
|
||||
monkeypatch.setenv("WIKI_AUTHOR", "Torben")
|
||||
name = "gateway.example.net"
|
||||
_fixture_raw_file(monkeypatch, kb_dir, "raw/notes/gateway.example.net.md")
|
||||
result = _invoke_new(monkeypatch, kb_dir, [
|
||||
"new", "source", "--name", name,
|
||||
"--set", "source_type=notes",
|
||||
"--set", "raw_files=raw/notes/gateway.example.net.md",
|
||||
"--set", "fidelity=verbatim", "--set", "authority=reporting",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
pattern = _path_pattern_from_note(_new_variant_notes("new source "), name)
|
||||
written = next(kb_dir.glob(f"sources/**/Source - {name}.md"))
|
||||
assert pattern.match(str(written.relative_to(kb_dir)))
|
||||
|
||||
|
||||
def test_new_comparison_path_matches_its_record_note(monkeypatch, kb_dir):
|
||||
"""`comparison` has no `layout:`, so its note names a literal path with
|
||||
no placeholder - the name used here is exactly what that literal names,
|
||||
which is what makes this case a genuine check rather than a tautology:
|
||||
a `layout:` added to `types/comparison.md` without updating the note
|
||||
would make the literal not match the now-nested actual path."""
|
||||
name = "X vs Y"
|
||||
result = _invoke_new(monkeypatch, kb_dir, [
|
||||
"new", "comparison", "--name", name, "--set", "entities=aurora,Borealis",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
pattern = _path_pattern_from_note(_new_variant_notes("new comparison "), name)
|
||||
written = kb_dir / "comparisons" / f"{name}.md"
|
||||
assert written.exists()
|
||||
assert pattern.match(str(written.relative_to(kb_dir)))
|
||||
|
||||
|
||||
def test_new_project_path_matches_its_record_note(monkeypatch, kb_dir):
|
||||
name = "Testvorhaben"
|
||||
result = _invoke_new(monkeypatch, kb_dir, [
|
||||
"new", "project", "--name", name, "--set", "responsibility=haus",
|
||||
])
|
||||
assert result.exit_code == 0, result.output
|
||||
pattern = _path_pattern_from_note(_new_variant_notes("new project "), name)
|
||||
written = next(kb_dir.glob(f"gtd/**/{name}.md"))
|
||||
assert pattern.match(str(written.relative_to(kb_dir)))
|
||||
|
||||
|
||||
def test_repeated_set_appends_for_array_fields():
|
||||
"""The separator-free way to pass an element containing a comma. The
|
||||
schema type decides: only array fields append."""
|
||||
|
||||
Reference in new issue
Block a user