Refactor buildzip to simplify make_nuwiki callback handling and improve test coverage

Christoph Kepper · Christoph Kepper · commit abbc93ecf13a · 2026-03-03T23:17:20.000+01:00
- Inline `_make_zip_callback` logic directly into `buildzip` for cleaner flow.
- Add a regression test to ensure `buildzip` zips the correct directory populated by `make_nuwiki`.
diff --git a/src/mwlib/apps/buildzip.py b/src/mwlib/apps/buildzip.py
@@ -388,7 +388,16 @@ def _build_zip(
         nuwiki_dir = os.path.join(tmpdir, "nuwiki")
         log.info(f"creating nuwiki in {nuwiki_dir!r}")
 
-        status = create_zip_from_wiki_env(env, pod_client, wiki_options, self._make_zip_callback)
+        def build_nuwiki_to_tmpdir(wiki_options, metabook, pod_client, status):
+            make_nuwiki(
+                fsdir=nuwiki_dir,
+                metabook=metabook,
+                wiki_options=wiki_options,
+                pod_client=pod_client,
+                status=status,
+            )
+
+        status = create_zip_from_wiki_env(env, pod_client, wiki_options, build_nuwiki_to_tmpdir)
 
         zip_path = ZipCreator.create_zip(nuwiki_dir, self.config.output)
 
@@ -398,22 +407,6 @@ def _build_zip(
 
         return zip_path
 
-    def _make_zip_callback(
-        self, wiki_options: dict, metabook: Any, pod_client: Any, status: Any
-    ) -> str:
-        """Callback for create_zip_from_wiki_env."""
-        # This is called by create_zip_from_wiki_env to create nuwiki
-        tmpdir = tempfile.mkdtemp()
-        fsdir = os.path.join(tmpdir, "nuwiki")
-        make_nuwiki(
-            fsdir=fsdir,
-            metabook=metabook,
-            wiki_options=wiki_options,
-            pod_client=pod_client,
-            status=status,
-        )
-        return fsdir
-
 
 # ============================================================================
 # CLI Entry Point (Thin Wrapper)
diff --git a/tests/mwlib/apps/test_buildzip.py b/tests/mwlib/apps/test_buildzip.py
@@ -1,5 +1,6 @@
 import os
 import tempfile
+import zipfile
 
 import pytest
 from click.testing import CliRunner
@@ -415,3 +416,60 @@ def test_build_config_immutable():
     # Should not be able to modify
     with pytest.raises(Exception):
         config.output = "other.zip"
+
+
+def test_zip_builder_build_zip_writes_and_zips_same_nuwiki_dir(monkeypatch, tmp_path):
+    """Regression: _build_zip must zip the directory that make_nuwiki populated."""
+    from types import SimpleNamespace
+
+    from mwlib.apps.buildzip import BuildConfig, ZipBuilder
+
+    output_zip = tmp_path / "out.zip"
+    build_config = BuildConfig(
+        output=str(output_zip),
+        posturl=None,
+        getposturl=0,
+        keep_tmpfiles=False,
+        status_file=None,
+        config=None,
+        imagesize=1280,
+        metabook=object(),
+        collectionpage=None,
+        noimages=False,
+        logfile=None,
+        username=None,
+        password=None,
+        domain=None,
+        title=None,
+        subtitle=None,
+        editor=None,
+        script_extension=".php",
+    )
+    builder = ZipBuilder(build_config)
+
+    def fake_create_zip_from_wiki_env(env, pod_client, wiki_options, make_zip):
+        make_zip(
+            wiki_options=wiki_options,
+            metabook=env.metabook,
+            pod_client=pod_client,
+            status=lambda **kwargs: None,
+        )
+        return lambda **kwargs: None
+
+    def fake_make_nuwiki(fsdir, metabook, wiki_options, pod_client, status):
+        os.makedirs(fsdir, exist_ok=True)
+        with open(os.path.join(fsdir, "marker.txt"), "w", encoding="utf-8") as f:
+            f.write("ok")
+
+    monkeypatch.setattr("mwlib.apps.buildzip.create_zip_from_wiki_env", fake_create_zip_from_wiki_env)
+    monkeypatch.setattr("mwlib.apps.buildzip.make_nuwiki", fake_make_nuwiki)
+
+    env = SimpleNamespace(metabook=object())
+    tmpdir = tmp_path / "tmp"
+    tmpdir.mkdir()
+
+    zip_path = builder._build_zip(str(tmpdir), env, {}, pod_client=None)
+
+    assert zip_path == str(output_zip)
+    with zipfile.ZipFile(zip_path, "r") as zf:
+        assert "marker.txt" in zf.namelist()
