[iris] Address PR review: drop redundant _db.close, bump pytest, harden teardown

- Remove six vestigial controller._db.close() calls in test_checkpoint.py now
  that the make_controller fixture stops the controller (which closes the DB)
  at teardown. Each redundant close blocked for ~1s draining the read pool.
- Bump pytest>=8.4 in lib/iris dev deps so truncation_limit_lines/chars aren't
  silently dropped on fresh resolves that pick 8.3.x.
- Make the make_controller teardown robust to per-controller stop() failures:
  collect errors, stop every controller, then re-raise.
- Expand the make_controller docstring to cover db= / provider= injection.
- Alphabetize the iris.cluster.controller.* import block in conftest.py.

Co-authored-by: Russell Power <rjpower@users.noreply.github.com>

Author: github-actions[bot]

lib/iris/pyproject.toml

@@ -63,7 +63,7 @@ dev = [
     "pytest-playwright>=0.6.2",
     "pytest-timeout",
     "pytest-xdist",
-    "pytest>=8.3.2",
+    "pytest>=8.4",
 ]
 
 [tool.hatch.build.hooks.custom]

lib/iris/tests/cluster/controller/conftest.py

@@ -28,13 +28,16 @@
 )
 from iris.cluster.controller.autoscaler import Autoscaler
 from iris.cluster.controller.autoscaler.models import DemandEntry
+from iris.cluster.controller.autoscaler.scaling_group import ScalingGroup
+from iris.cluster.controller.controller import Controller, ControllerConfig
 from iris.cluster.controller.db import (
     ACTIVE_TASK_STATES,
     ControllerDB,
     _decode_attribute_rows,
     task_row_can_be_scheduled,
     task_row_is_finished,
 )
+from iris.cluster.controller.provider import ProviderUnsupportedError
 from iris.cluster.controller.schema import (
     ATTEMPT_PROJECTION,
     JOB_CONFIG_JOIN,
@@ -47,9 +50,6 @@
     WorkerRow,
     tasks_with_attempts,
 )
-from iris.cluster.controller.controller import Controller, ControllerConfig
-from iris.cluster.controller.provider import ProviderUnsupportedError
-from iris.cluster.controller.autoscaler.scaling_group import ScalingGroup
 from iris.cluster.controller.service import ControllerServiceImpl
 from iris.log_server.server import LogServiceImpl
 from iris.cluster.controller.transitions import (
@@ -213,11 +213,21 @@ def make_controller(tmp_path):
     monkeypatched ``LogServiceClientSync``. The factory tracks every
     constructed controller and ``stop()``s them at fixture teardown.
 
+    Pass ``db=`` to inject a pre-built ``ControllerDB`` (otherwise the
+    ``Controller`` opens one under ``config.local_state_dir``). Pass
+    ``provider=`` to override the default ``FakeProvider``. Any remaining
+    keyword arguments are forwarded to ``ControllerConfig``.
+
     Usage::
 
-        def test_foo(make_controller):
+        def test_foo(make_controller, tmp_path):
             ctrl = make_controller(remote_state_dir="file:///tmp/iris-state")
-            ...
+            # Or inject an existing DB / provider:
+            ctrl = make_controller(
+                remote_state_dir="file:///tmp/iris-state",
+                local_state_dir=tmp_path,
+                db=my_db,
+            )
     """
     created: list[Controller] = []
 
@@ -242,8 +252,14 @@ def _factory(
         return controller
 
     yield _factory
+    errors: list[BaseException] = []
     for controller in created:
-        controller.stop()
+        try:
+            controller.stop()
+        except BaseException as exc:
+            errors.append(exc)
+    if errors:
+        raise errors[0]
 
 
 def make_test_entrypoint() -> job_pb2.RuntimeEntrypoint:

lib/iris/tests/cluster/controller/test_checkpoint.py

@@ -31,8 +31,6 @@ def test_write_checkpoint_uploads_compressed(tmp_path, make_controller):
     assert result.task_count == 0
     assert result.worker_count == 0
 
-    controller._db.close()
-
 
 def test_begin_checkpoint_returns_remote_path(tmp_path, make_controller):
     """begin_checkpoint returns a remote path string."""
@@ -44,8 +42,6 @@ def test_begin_checkpoint_returns_remote_path(tmp_path, make_controller):
     assert path.startswith(f"file://{tmp_path}/remote/controller-state/")
     assert result.job_count == 0
 
-    controller._db.close()
-
 
 def test_atexit_checkpoint_writes_to_remote(tmp_path, make_controller):
     """_atexit_checkpoint writes directly to remote storage."""
@@ -59,8 +55,6 @@ def test_atexit_checkpoint_writes_to_remote(tmp_path, make_controller):
     assert len(timestamped_dirs) >= 1
     assert (timestamped_dirs[0] / "controller.sqlite3.zst").exists()
 
-    controller._db.close()
-
 
 def test_download_checkpoint_to_local(tmp_path):
     """download_checkpoint_to_local copies remote DB to local path."""
@@ -101,7 +95,6 @@ def test_write_checkpoint_roundtrip(tmp_path, make_controller):
     remote_dir = f"file://{tmp_path}/remote"
     controller = make_controller(remote_state_dir=remote_dir)
     write_checkpoint(controller._db, remote_dir)
-    controller._db.close()
 
     local_db_dir = tmp_path / "restored"
     download_checkpoint_to_local(remote_dir, local_db_dir)
@@ -123,8 +116,6 @@ def test_write_checkpoint_cleans_up_temp_file(tmp_path, make_controller):
     sqlite_temps = [f for f in new_files if ".sqlite3" in f.name and f.name != ControllerDB.DB_FILENAME]
     assert len(sqlite_temps) == 0
 
-    controller._db.close()
-
 
 def test_local_db_exists_skips_remote_download(tmp_path):
     """When a local DB already exists, download_checkpoint_to_local should not be called.
@@ -213,8 +204,6 @@ def test_periodic_checkpoint_inline(tmp_path, make_controller):
     assert len(timestamped_dirs) >= 1
     assert (timestamped_dirs[0] / "controller.sqlite3.zst").exists()
 
-    controller._db.close()
-
 
 def test_download_uncompressed_fallback(tmp_path):
     """download_checkpoint_to_local falls back to uncompressed files from old checkpoints."""