mirror of
https://github.com/brunopostle/ifcurl.git
synced 2026-07-12 10:18:14 +00:00
tests: add TestCacheCli covering list, clear, and prune CLI paths
9 tests exercise _cmd_cache() directly with fake cache dirs and a monkeypatched user_cache_dir, covering output formatting, empty states, prune with --max-gb arg, prune via IFCURL_CACHE_MAX_GB env, and the error exit when neither is set. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
ab4fb6ded9
commit
4df69d229d
2 changed files with 110 additions and 1 deletions
|
|
@ -58,7 +58,7 @@
|
||||||
{"id":"ifcurl-z4f","title":"render.py: concurrent requests each spawn full CPU-count process pools","description":"render() calls ifcopenshell.geom.iterator(settings, model, multiprocessing.cpu_count(), ...). In a multi-request service scenario, N concurrent renders each spawn cpu_count worker processes, so a 16-core machine handling 4 simultaneous requests spawns 64 processes. This can saturate the machine and is effectively a DoS amplifier. Consider capping the worker count (e.g. max 2 or 4 per request), or using a shared process pool at the service level.","status":"closed","priority":3,"issue_type":"bug","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T10:41:16Z","created_by":"Bruno Postle","updated_at":"2026-04-24T15:52:14Z","started_at":"2026-04-24T14:47:54Z","closed_at":"2026-04-24T15:52:14Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
|
{"id":"ifcurl-z4f","title":"render.py: concurrent requests each spawn full CPU-count process pools","description":"render() calls ifcopenshell.geom.iterator(settings, model, multiprocessing.cpu_count(), ...). In a multi-request service scenario, N concurrent renders each spawn cpu_count worker processes, so a 16-core machine handling 4 simultaneous requests spawns 64 processes. This can saturate the machine and is effectively a DoS amplifier. Consider capping the worker count (e.g. max 2 or 4 per request), or using a shared process pool at the service level.","status":"closed","priority":3,"issue_type":"bug","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T10:41:16Z","created_by":"Bruno Postle","updated_at":"2026-04-24T15:52:14Z","started_at":"2026-04-24T14:47:54Z","closed_at":"2026-04-24T15:52:14Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
|
||||||
{"id":"ifcurl-5or","title":"README.md: library usage example doesn't show temp file pattern for ifcopenshell.open()","description":"The 'Use as a library' section shows 'model = ifcopenshell.open(...) # write ifc_bytes to tmp file first' with a comment but no actual code. ifcopenshell.open() requires a file path, not bytes. A new user following the README will be stuck. The example should show the tempfile.mkstemp pattern used in _load_model() in service.py, or at least reference that function.","status":"closed","priority":3,"issue_type":"task","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T10:41:04Z","created_by":"Bruno Postle","updated_at":"2026-04-24T15:52:14Z","started_at":"2026-04-24T14:47:55Z","closed_at":"2026-04-24T15:52:14Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
|
{"id":"ifcurl-5or","title":"README.md: library usage example doesn't show temp file pattern for ifcopenshell.open()","description":"The 'Use as a library' section shows 'model = ifcopenshell.open(...) # write ifc_bytes to tmp file first' with a comment but no actual code. ifcopenshell.open() requires a file path, not bytes. A new user following the README will be stuck. The example should show the tempfile.mkstemp pattern used in _load_model() in service.py, or at least reference that function.","status":"closed","priority":3,"issue_type":"task","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T10:41:04Z","created_by":"Bruno Postle","updated_at":"2026-04-24T15:52:14Z","started_at":"2026-04-24T14:47:55Z","closed_at":"2026-04-24T15:52:14Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
|
||||||
{"id":"ifcurl-dyy","title":"viewer.html: BCF export from viewer omits component GUIDs — undocumented","description":"The viewer's client-side BCF export cannot include component GUIDs because it has no access to the IFC model's resolved GlobalIds. The service POST /bcf endpoint resolves the selector server-side and produces proper BCF with \u003cSelection\u003e or \u003cVisibility/Exceptions\u003e elements. This discrepancy is not documented anywhere. The viewer's BCF button should either show a note ('GUIDs not resolved — use /bcf API for full component selection') or redirect to the service endpoint when available.","status":"closed","priority":3,"issue_type":"task","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T10:40:49Z","created_by":"Bruno Postle","updated_at":"2026-04-25T11:54:29Z","started_at":"2026-04-25T11:32:57Z","closed_at":"2026-04-25T11:54:29Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
|
{"id":"ifcurl-dyy","title":"viewer.html: BCF export from viewer omits component GUIDs — undocumented","description":"The viewer's client-side BCF export cannot include component GUIDs because it has no access to the IFC model's resolved GlobalIds. The service POST /bcf endpoint resolves the selector server-side and produces proper BCF with \u003cSelection\u003e or \u003cVisibility/Exceptions\u003e elements. This discrepancy is not documented anywhere. The viewer's BCF button should either show a note ('GUIDs not resolved — use /bcf API for full component selection') or redirect to the service endpoint when available.","status":"closed","priority":3,"issue_type":"task","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T10:40:49Z","created_by":"Bruno Postle","updated_at":"2026-04-25T11:54:29Z","started_at":"2026-04-25T11:32:57Z","closed_at":"2026-04-25T11:54:29Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
|
||||||
{"id":"ifcurl-tq8","title":"No tests for ifcurl cache CLI subcommands","description":"The 'ifcurl cache list', 'ifcurl cache prune', and 'ifcurl cache clear' subcommands in __main__.py have no automated test coverage. _repo_cache_entries(), _evict_if_needed(), and _get_max_cache_bytes() have unit tests in test_git.py, but the CLI layer (_cmd_cache) is untested. Should add tests that exercise the argparse wiring, the output formatting, and the prune logic via subprocess or by calling _cmd_cache() directly with a fake args namespace.","status":"open","priority":3,"issue_type":"task","owner":"bruno@postle.net","created_at":"2026-04-24T10:40:45Z","created_by":"Bruno Postle","updated_at":"2026-04-24T10:40:45Z","dependency_count":0,"dependent_count":0,"comment_count":0}
|
{"id":"ifcurl-tq8","title":"No tests for ifcurl cache CLI subcommands","description":"The 'ifcurl cache list', 'ifcurl cache prune', and 'ifcurl cache clear' subcommands in __main__.py have no automated test coverage. _repo_cache_entries(), _evict_if_needed(), and _get_max_cache_bytes() have unit tests in test_git.py, but the CLI layer (_cmd_cache) is untested. Should add tests that exercise the argparse wiring, the output formatting, and the prune logic via subprocess or by calling _cmd_cache() directly with a fake args namespace.","status":"in_progress","priority":3,"issue_type":"task","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T10:40:45Z","created_by":"Bruno Postle","updated_at":"2026-04-25T19:33:40Z","started_at":"2026-04-25T19:33:40Z","dependency_count":0,"dependent_count":0,"comment_count":0}
|
||||||
{"id":"ifcurl-ovi","title":"dev/compose.yml uses Forgejo v10, but go.patch targets v13","description":"dev/compose.yml pulls codeberg.org/forgejo/forgejo:10 for local development. The go.patch and ifc_url.go were written and tested against the v13 branch. Running the dev environment with v10 won't have the patched binary or the correct template slots. The compose file should either use the patched v13 build, or document that the stock container is for testing the service/viewer in isolation only, not for testing the Go patch.","status":"closed","priority":3,"issue_type":"bug","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T10:40:35Z","created_by":"Bruno Postle","updated_at":"2026-04-25T09:05:41Z","started_at":"2026-04-25T09:05:21Z","closed_at":"2026-04-25T09:05:41Z","close_reason":"Updated image to v13 and added comment clarifying stock container doesn't include the IFC patch","dependency_count":0,"dependent_count":0,"comment_count":0}
|
{"id":"ifcurl-ovi","title":"dev/compose.yml uses Forgejo v10, but go.patch targets v13","description":"dev/compose.yml pulls codeberg.org/forgejo/forgejo:10 for local development. The go.patch and ifc_url.go were written and tested against the v13 branch. Running the dev environment with v10 won't have the patched binary or the correct template slots. The compose file should either use the patched v13 build, or document that the stock container is for testing the service/viewer in isolation only, not for testing the Go patch.","status":"closed","priority":3,"issue_type":"bug","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T10:40:35Z","created_by":"Bruno Postle","updated_at":"2026-04-25T09:05:41Z","started_at":"2026-04-25T09:05:21Z","closed_at":"2026-04-25T09:05:41Z","close_reason":"Updated image to v13 and added comment clarifying stock container doesn't include the IFC patch","dependency_count":0,"dependent_count":0,"comment_count":0}
|
||||||
{"id":"ifcurl-j35","title":"git.py: silent failure on mutable ref fetch serves stale data without warning","description":"In _open_remote(), when fetching for a mutable ref (branch/HEAD) fails — network error, auth failure, server down — the exception is caught and ignored ('pass # offline — use cached data'). The caller receives stale data with no indication anything went wrong. The render then proceeds, potentially showing outdated geometry. At minimum a log.warning should be emitted. Ideally the service response should include a header (e.g. X-Ifcurl-Cache: stale) so Forgejo or the browser viewer can surface a warning to the user.","status":"closed","priority":3,"issue_type":"bug","owner":"bruno@postle.net","created_at":"2026-04-24T10:40:31Z","created_by":"Bruno Postle","updated_at":"2026-04-25T08:05:18Z","closed_at":"2026-04-25T08:05:18Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
|
{"id":"ifcurl-j35","title":"git.py: silent failure on mutable ref fetch serves stale data without warning","description":"In _open_remote(), when fetching for a mutable ref (branch/HEAD) fails — network error, auth failure, server down — the exception is caught and ignored ('pass # offline — use cached data'). The caller receives stale data with no indication anything went wrong. The render then proceeds, potentially showing outdated geometry. At minimum a log.warning should be emitted. Ideally the service response should include a header (e.g. X-Ifcurl-Cache: stale) so Forgejo or the browser viewer can surface a warning to the user.","status":"closed","priority":3,"issue_type":"bug","owner":"bruno@postle.net","created_at":"2026-04-24T10:40:31Z","created_by":"Bruno Postle","updated_at":"2026-04-25T08:05:18Z","closed_at":"2026-04-25T08:05:18Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
|
||||||
{"id":"ifcurl-69l","title":"License headers for Forgejo integration files","description":"The Go patch files (ifc_url.go, ifc_url_test.go) already carry MIT headers matching Forgejo's per-file convention. The JS/HTML assets have no license header: viewer.html, viewer-url.js, footer.tmpl. Review: (1) confirm MIT is correct for files intended for Forgejo contribution (Forgejo project is GPL-3.0 but individual contributed files use MIT); (2) check compatibility with ThatOpen components licence (viewer.html loads @thatopen/components from CDN); (3) add SPDX licence headers to the three unlicensed asset files. The Python/CLI core remains LGPL-3.0-or-later as it targets IfcOpenShell contribution.","status":"closed","priority":3,"issue_type":"task","owner":"bruno@postle.net","created_at":"2026-04-24T07:09:46Z","created_by":"Bruno Postle","updated_at":"2026-04-24T07:29:22Z","closed_at":"2026-04-24T07:29:22Z","close_reason":"Added MIT SPDX headers to viewer.html, viewer-url.js, footer.tmpl — ThatOpen components also MIT-licensed so no compatibility issue","dependency_count":0,"dependent_count":0,"comment_count":0}
|
{"id":"ifcurl-69l","title":"License headers for Forgejo integration files","description":"The Go patch files (ifc_url.go, ifc_url_test.go) already carry MIT headers matching Forgejo's per-file convention. The JS/HTML assets have no license header: viewer.html, viewer-url.js, footer.tmpl. Review: (1) confirm MIT is correct for files intended for Forgejo contribution (Forgejo project is GPL-3.0 but individual contributed files use MIT); (2) check compatibility with ThatOpen components licence (viewer.html loads @thatopen/components from CDN); (3) add SPDX licence headers to the three unlicensed asset files. The Python/CLI core remains LGPL-3.0-or-later as it targets IfcOpenShell contribution.","status":"closed","priority":3,"issue_type":"task","owner":"bruno@postle.net","created_at":"2026-04-24T07:09:46Z","created_by":"Bruno Postle","updated_at":"2026-04-24T07:29:22Z","closed_at":"2026-04-24T07:29:22Z","close_reason":"Added MIT SPDX headers to viewer.html, viewer-url.js, footer.tmpl — ThatOpen components also MIT-licensed so no compatibility issue","dependency_count":0,"dependent_count":0,"comment_count":0}
|
||||||
|
|
|
||||||
|
|
@ -178,3 +178,112 @@ class TestCacheHelpers:
|
||||||
entry = _make_fake_repo_cache(tmp_path, "https://example.com/repo")
|
entry = _make_fake_repo_cache(tmp_path, "https://example.com/repo")
|
||||||
_evict_if_needed()
|
_evict_if_needed()
|
||||||
assert entry.exists()
|
assert entry.exists()
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Cache CLI (_cmd_cache)
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
import argparse
|
||||||
|
import types
|
||||||
|
|
||||||
|
|
||||||
|
def _cache_args(**kwargs) -> argparse.Namespace:
|
||||||
|
defaults = {"cache_cmd": None, "max_gb": None}
|
||||||
|
defaults.update(kwargs)
|
||||||
|
return argparse.Namespace(**defaults)
|
||||||
|
|
||||||
|
|
||||||
|
class TestCacheCli:
|
||||||
|
def _patch(self, monkeypatch, tmp_path):
|
||||||
|
monkeypatch.setattr("ifcurl.git.user_cache_dir", lambda *a, **kw: str(tmp_path))
|
||||||
|
monkeypatch.setattr("ifcurl.__main__.user_cache_dir", lambda *a, **kw: str(tmp_path), raising=False)
|
||||||
|
|
||||||
|
def test_list_empty(self, tmp_path, monkeypatch, capsys):
|
||||||
|
self._patch(monkeypatch, tmp_path)
|
||||||
|
from ifcurl.__main__ import _cmd_cache
|
||||||
|
_cmd_cache(_cache_args(cache_cmd="list"))
|
||||||
|
out = capsys.readouterr().out
|
||||||
|
assert "No cached repositories" in out
|
||||||
|
|
||||||
|
def test_list_default_subcommand(self, tmp_path, monkeypatch, capsys):
|
||||||
|
self._patch(monkeypatch, tmp_path)
|
||||||
|
_make_fake_repo_cache(tmp_path, "https://example.com/org/repo", size_bytes=2048)
|
||||||
|
from ifcurl.__main__ import _cmd_cache
|
||||||
|
_cmd_cache(_cache_args(cache_cmd=None))
|
||||||
|
out = capsys.readouterr().out
|
||||||
|
assert "https://example.com/org/repo" in out
|
||||||
|
assert "Total:" in out
|
||||||
|
|
||||||
|
def test_list_shows_entries_and_total(self, tmp_path, monkeypatch, capsys):
|
||||||
|
self._patch(monkeypatch, tmp_path)
|
||||||
|
monkeypatch.delenv("IFCURL_CACHE_MAX_GB", raising=False)
|
||||||
|
_make_fake_repo_cache(tmp_path, "https://example.com/a", size_bytes=1024 * 1024)
|
||||||
|
_make_fake_repo_cache(tmp_path, "https://example.com/b", size_bytes=1024 * 1024)
|
||||||
|
from ifcurl.__main__ import _cmd_cache
|
||||||
|
_cmd_cache(_cache_args(cache_cmd="list"))
|
||||||
|
out = capsys.readouterr().out
|
||||||
|
assert "https://example.com/a" in out
|
||||||
|
assert "https://example.com/b" in out
|
||||||
|
assert "2 repos" in out
|
||||||
|
|
||||||
|
def test_list_shows_limit_when_env_set(self, tmp_path, monkeypatch, capsys):
|
||||||
|
self._patch(monkeypatch, tmp_path)
|
||||||
|
monkeypatch.setenv("IFCURL_CACHE_MAX_GB", "5.0")
|
||||||
|
_make_fake_repo_cache(tmp_path, "https://example.com/a", size_bytes=1024)
|
||||||
|
from ifcurl.__main__ import _cmd_cache
|
||||||
|
_cmd_cache(_cache_args(cache_cmd="list"))
|
||||||
|
out = capsys.readouterr().out
|
||||||
|
assert "limit:" in out
|
||||||
|
assert "5.0 GB" in out
|
||||||
|
|
||||||
|
def test_clear_removes_all(self, tmp_path, monkeypatch, capsys):
|
||||||
|
self._patch(monkeypatch, tmp_path)
|
||||||
|
e1 = _make_fake_repo_cache(tmp_path, "https://example.com/a")
|
||||||
|
e2 = _make_fake_repo_cache(tmp_path, "https://example.com/b")
|
||||||
|
from ifcurl.__main__ import _cmd_cache
|
||||||
|
_cmd_cache(_cache_args(cache_cmd="clear"))
|
||||||
|
assert not e1.exists()
|
||||||
|
assert not e2.exists()
|
||||||
|
out = capsys.readouterr().out
|
||||||
|
assert "Cleared" in out
|
||||||
|
assert "2 repos" in out
|
||||||
|
|
||||||
|
def test_clear_empty(self, tmp_path, monkeypatch, capsys):
|
||||||
|
self._patch(monkeypatch, tmp_path)
|
||||||
|
from ifcurl.__main__ import _cmd_cache
|
||||||
|
_cmd_cache(_cache_args(cache_cmd="clear"))
|
||||||
|
out = capsys.readouterr().out
|
||||||
|
assert "Nothing to clear" in out
|
||||||
|
|
||||||
|
def test_prune_with_max_gb_arg(self, tmp_path, monkeypatch, capsys):
|
||||||
|
self._patch(monkeypatch, tmp_path)
|
||||||
|
monkeypatch.delenv("IFCURL_CACHE_MAX_GB", raising=False)
|
||||||
|
# Two 1 MB entries; limit to 0.5 MB so both are evicted
|
||||||
|
e1 = _make_fake_repo_cache(tmp_path, "https://example.com/old", size_bytes=1024 * 1024)
|
||||||
|
time.sleep(0.05)
|
||||||
|
e2 = _make_fake_repo_cache(tmp_path, "https://example.com/new", size_bytes=1024 * 1024)
|
||||||
|
from ifcurl.__main__ import _cmd_cache
|
||||||
|
_cmd_cache(_cache_args(cache_cmd="prune", max_gb=0.0000005))
|
||||||
|
out = capsys.readouterr().out
|
||||||
|
assert "Freed" in out
|
||||||
|
assert "remaining" in out
|
||||||
|
|
||||||
|
def test_prune_no_max_gb_and_no_env_exits(self, tmp_path, monkeypatch, capsys):
|
||||||
|
self._patch(monkeypatch, tmp_path)
|
||||||
|
monkeypatch.delenv("IFCURL_CACHE_MAX_GB", raising=False)
|
||||||
|
from ifcurl.__main__ import _cmd_cache
|
||||||
|
with pytest.raises(SystemExit):
|
||||||
|
_cmd_cache(_cache_args(cache_cmd="prune", max_gb=None))
|
||||||
|
err = capsys.readouterr().err
|
||||||
|
assert "required" in err
|
||||||
|
|
||||||
|
def test_prune_uses_env_when_no_arg(self, tmp_path, monkeypatch, capsys):
|
||||||
|
self._patch(monkeypatch, tmp_path)
|
||||||
|
monkeypatch.setenv("IFCURL_CACHE_MAX_GB", "0.0000005")
|
||||||
|
_make_fake_repo_cache(tmp_path, "https://example.com/repo", size_bytes=1024 * 1024)
|
||||||
|
from ifcurl.__main__ import _cmd_cache
|
||||||
|
_cmd_cache(_cache_args(cache_cmd="prune", max_gb=None))
|
||||||
|
out = capsys.readouterr().out
|
||||||
|
assert "Freed" in out
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue