fix(ofrep): stop leaking the testbed when it never becomes ready

Two leaks on the same path, both reported by coderabbit on #414.

running_testbed called start() outside the try, and start() brings the compose
stack up before waiting for readiness. So a readiness timeout left containers
running that nothing would stop -- and a suite that cannot reach its backend is
exactly when someone runs it again, against the stack the last run abandoned.

stop() also never removed the temporary directory that __init__ creates for the
bind mount, so every constructed testbed left one behind whether or not anything
failed. It now comes off in a finally, because a compose failure is precisely
when it would otherwise be missed.

Verified on the failing path rather than by inspection: with readiness forced to
raise, the temporary directory is removed and no containers remain. The suite is
unchanged at 23 passed, 5 skipped, 1 xfailed.

Signed-off-by: Simon Schrottner <simon.schrottner@flagsmith.com>

Author: aepfli

providers/openfeature-provider-ofrep/tests/tck/testbed.py

@@ -27,6 +27,7 @@
 from __future__ import annotations
 
 import os
+import shutil
 import tempfile
 import time
 import typing
@@ -121,7 +122,16 @@ def start(self) -> FlagdTestbed:
         return self
 
     def stop(self) -> None:
-        self._compose.stop()
+        """Stop the stack and remove the temporary flag directory.
+
+        The directory comes off in a ``finally`` because a compose failure is
+        exactly when it would otherwise be left behind, and it is created in
+        ``__init__`` -- so every constructed testbed leaks one until this runs.
+        """
+        try:
+            self._compose.stop()
+        finally:
+            shutil.rmtree(self._flags_dir, ignore_errors=True)
 
     def get_ofrep_url(self) -> str:
         """Return the base URL to hand to ``OFREPProvider``.
@@ -163,10 +173,16 @@ def _await_ready(self) -> None:
 
 
 def running_testbed() -> typing.Iterator[FlagdTestbed]:
-    """Yield a started testbed and stop it afterwards. Used by the session fixture."""
+    """Yield a started testbed and stop it afterwards. Used by the session fixture.
+
+    ``start`` is inside the ``try`` on purpose. It brings the stack up and then
+    waits for readiness, so a readiness timeout leaves containers running that
+    nothing would otherwise stop -- and a suite that cannot reach its backend is
+    precisely when a developer is going to run it again.
+    """
     testbed = FlagdTestbed()
-    testbed.start()
     try:
+        testbed.start()
         yield testbed
     finally:
         testbed.stop()