Ban porcelain/cli in lower level tests (#2060)

Author: jelmer

dulwich/client.py

@@ -1478,12 +1478,7 @@ def archive(
 
     @staticmethod
     def _warn_filter_objects() -> None:
-        import warnings
-
-        warnings.warn(
-            "object filtering not recognized by server, ignoring",
-            UserWarning,
-        )
+        logging.warning("object filtering not recognized by server, ignoring")
 
 
 def check_wants(wants: Set[bytes], refs: Mapping[bytes, bytes]) -> None:

dulwich/signature.py

@@ -1293,12 +1293,13 @@ def verify(self, data: bytes, signature: bytes) -> None:
                 args.extend(["-r", self.revocation_file])
 
             try:
-                subprocess.run(
-                    args,
-                    stdin=open(data_filename, "rb"),
-                    capture_output=True,
-                    check=True,
-                )
+                with open(data_filename, "rb") as data_file:
+                    subprocess.run(
+                        args,
+                        stdin=data_file,
+                        capture_output=True,
+                        check=True,
+                    )
             except subprocess.CalledProcessError as e:
                 raise BadSignature(
                     f"SSH signature verification failed: {e.stderr.decode('utf-8', errors='replace')}"

tests/__init__.py

@@ -132,9 +132,6 @@ def self_test_suite() -> unittest.TestSuite:
         "bitmap",
         "blackbox",
         "bundle",
-        "cli",
-        "cli_cherry_pick",
-        "cli_merge",
         "client",
         "cloud_gcs",
         "commit_graph",
@@ -197,12 +194,24 @@ def self_test_suite() -> unittest.TestSuite:
         "worktree",
     ]
     module_names = ["tests.test_" + name for name in names]
+    cli_names = [
+        "cherry_pick",
+        "cli",
+        "merge",
+    ]
+    module_names += ["tests.cli.test_" + name for name in cli_names]
     porcelain_names = [
+        "annotate",
+        "bisect",
         "cherry_pick",
         "filters",
+        "ignore",
         "lfs",
+        "maintenance",
+        "mbox",
         "merge",
         "notes",
+        "rebase",
     ]
     module_names += ["tests.porcelain"] + [
         "tests.porcelain.test_" + name for name in porcelain_names

tests/cli/__init__.py

@@ -27,7 +27,7 @@
 from dulwich import porcelain
 from dulwich.cli import cmd_cherry_pick
 
-from . import TestCase
+from .. import TestCase
 
 
 class CherryPickCommandTests(TestCase):

tests/test_cli_cherry_pick.py tests/cli/test_cherry_pick.pytests/test_cli_cherry_pick.py renamed to tests/cli/test_cherry_pick.py

@@ -24,6 +24,7 @@
 """Tests for dulwich.cli."""
 
 import io
+import logging
 import os
 import shutil
 import sys
@@ -47,14 +48,25 @@
     build_commit_graph,
 )
 
-from . import TestCase
+from .. import TestCase
 
 
 class DulwichCliTestCase(TestCase):
     """Base class for CLI tests."""
 
     def setUp(self) -> None:
         super().setUp()
+        # Suppress expected error logging during CLI tests
+        cli_logger = logging.getLogger("dulwich.cli")
+        original_cli_level = cli_logger.level
+        cli_logger.setLevel(logging.CRITICAL)
+        self.addCleanup(cli_logger.setLevel, original_cli_level)
+
+        root_logger = logging.getLogger()
+        original_root_level = root_logger.level
+        root_logger.setLevel(logging.CRITICAL)
+        self.addCleanup(root_logger.setLevel, original_root_level)
+
         self.test_dir = tempfile.mkdtemp()
         self.addCleanup(shutil.rmtree, self.test_dir)
         self.repo_path = os.path.join(self.test_dir, "repo")

tests/test_cli.py tests/cli/test_cli.pytests/test_cli.py renamed to tests/cli/test_cli.py

@@ -29,7 +29,7 @@
 from dulwich import porcelain
 from dulwich.cli import main
 
-from . import DependencyMissing, TestCase
+from .. import DependencyMissing, TestCase
 
 
 class CLIMergeTests(TestCase):

tests/test_cli_merge.py tests/cli/test_merge.pytests/test_cli_merge.py renamed to tests/cli/test_merge.py

@@ -390,14 +390,15 @@ def determine_wants(refs, depth=None):
             # Get all refs
             return list(refs.values())
 
-        # Fetch with filter
-        result = client.fetch(
-            path,
-            dest_repo,
-            determine_wants=determine_wants,
-            progress=None,
-            filter_spec=b"blob:none",
-        )
+        # Fetch with filter (may warn if server doesn't support filtering)
+        with self.assertLogs(level="WARNING"):
+            result = client.fetch(
+                path,
+                dest_repo,
+                determine_wants=determine_wants,
+                progress=None,
+                filter_spec=b"blob:none",
+            )
 
         # The fetch should succeed with partial clone
         self.assertIsNotNone(result)
@@ -430,13 +431,14 @@ def test_clone_with_filter(self) -> None:
 
         client, path = get_transport_and_path(f"git://localhost:{daemon_port}/")
 
-        # Clone with blob:limit filter
-        cloned_repo = client.clone(
-            path,
-            dest_path,
-            mkdir=False,
-            filter_spec=b"blob:limit=100",
-        )
+        # Clone with blob:limit filter (may warn if server doesn't support filtering)
+        with self.assertLogs(level="WARNING"):
+            cloned_repo = client.clone(
+                path,
+                dest_path,
+                mkdir=False,
+                filter_spec=b"blob:limit=100",
+            )
         self.addCleanup(cloned_repo.close)
 
         # Verify clone succeeded
@@ -483,6 +485,11 @@ def _start_git_daemon(self, repo_path):
         def cleanup_daemon():
             daemon_process.terminate()
             daemon_process.wait(timeout=2)
+            # Close pipes to avoid ResourceWarning
+            if daemon_process.stdout:
+                daemon_process.stdout.close()
+            if daemon_process.stderr:
+                daemon_process.stderr.close()
 
         self.addCleanup(cleanup_daemon)