refactor(test): replace deprecated tempfile.mktemp() with tempfile.mkstemp() #736

Closed
CoreRasurae wants to merge 1 commits from refactor/m3-replace-mktemp into master
18 changed files with 82 additions and 32 deletions
+5
View File
@@ -2,6 +2,11 @@
## Unreleased
- Replaced all 28 uses of deprecated `tempfile.mktemp()` with atomic
`tempfile.mkstemp()` + `os.close(fd)` across 16 test and benchmark files.
Eliminates TOCTOU race conditions, prevents potential path collisions under
parallel test execution, and removes Python deprecation warnings. All 10,640
BDD scenarios continue to pass under both sequential and parallel modes. (#730)
- Added TDD-style failing Behave BDD tests for the session list DI container
missing `db` provider bug. Three scenarios exercise `session list`,
`_get_session_service()`, and `session list --format json` through the real
+9 -4
View File
@@ -7,6 +7,7 @@ close-reopen cycle used in Robot persistence lifecycle tests.
from __future__ import annotations
import os
import tempfile
from datetime import UTC, datetime
from pathlib import Path
@@ -116,7 +117,8 @@ class TimeRobotFixtureSetupSuite:
def time_file_db_init(self) -> None:
"""Measure file-backed SQLite init + schema creation."""
tmp: str = tempfile.mktemp(suffix=".db")
fd, tmp = tempfile.mkstemp(suffix=".db")
os.close(fd)
try:
init_database(f"sqlite:///{tmp}")
finally:
@@ -124,7 +126,8 @@ class TimeRobotFixtureSetupSuite:
def time_seed_action(self) -> None:
"""Measure action seeding into a fresh file DB."""
tmp: str = tempfile.mktemp(suffix=".db")
fd, tmp = tempfile.mkstemp(suffix=".db")
os.close(fd)
try:
engine = init_database(f"sqlite:///{tmp}")
session = Session(bind=engine)
@@ -138,7 +141,8 @@ class TimeRobotFixtureSetupSuite:
def time_seed_plan(self) -> None:
"""Measure plan creation including FK action seeding."""
tmp: str = tempfile.mktemp(suffix=".db")
fd, tmp = tempfile.mkstemp(suffix=".db")
os.close(fd)
try:
engine = init_database(f"sqlite:///{tmp}")
session = Session(bind=engine)
@@ -161,7 +165,8 @@ class TimeRobotRestartCycleSuite:
def setup(self) -> None:
"""Create file DB with action + plan for restart benchmarks."""
self._tmp: str = tempfile.mktemp(suffix=".db")
fd, self._tmp = tempfile.mkstemp(suffix=".db")
os.close(fd)
engine = init_database(f"sqlite:///{self._tmp}")
session = Session(bind=engine)
factory = lambda: session # noqa: E731
+8 -7
View File
@@ -44,13 +44,13 @@ def before_all(context):
# Use per-process unique database paths so parallel test subprocesses
# (behave-parallel) never contend on the same SQLite file.
if "CLEVERAGENTS_DATABASE_URL" not in os.environ:
os.environ["CLEVERAGENTS_DATABASE_URL"] = (
f"sqlite:///{tempfile.mktemp(suffix='.db', prefix='cleveragents_')}"
)
_fd, _db_path = tempfile.mkstemp(suffix=".db", prefix="cleveragents_")
os.close(_fd)
os.environ["CLEVERAGENTS_DATABASE_URL"] = f"sqlite:///{_db_path}"
if "CLEVERAGENTS_TEST_DATABASE_URL" not in os.environ:
os.environ["CLEVERAGENTS_TEST_DATABASE_URL"] = (
f"sqlite:///{tempfile.mktemp(suffix='.db', prefix='cleveragents_test_')}"
)
_fd, _db_path = tempfile.mkstemp(suffix=".db", prefix="cleveragents_test_")
os.close(_fd)
os.environ["CLEVERAGENTS_TEST_DATABASE_URL"] = f"sqlite:///{_db_path}"
os.environ.setdefault("BEHAVE_TESTING", "true")
# Set up mock AI provider for all tests
@@ -289,7 +289,8 @@ def before_scenario(context, scenario):
("CLEVERAGENTS_DATABASE_URL", "cleveragents_"),
("CLEVERAGENTS_TEST_DATABASE_URL", "cleveragents_test_"),
):
db_path = tempfile.mktemp(suffix=".db", prefix=prefix)
_fd, db_path = tempfile.mkstemp(suffix=".db", prefix=prefix)
os.close(_fd)
os.environ[env_var] = f"sqlite:///{db_path}"
context._scenario_db_paths.append(db_path)
@@ -1,5 +1,6 @@
"""Step definitions for Automation Profile Guards tests."""
import os
import tempfile
from typing import Any
@@ -222,7 +223,8 @@ def step_yaml_file_with_guards(context: Context) -> None:
"require_approval_for_writes": True,
},
}
tmp_path = tempfile.mktemp(suffix=".yaml")
fd, tmp_path = tempfile.mkstemp(suffix=".yaml")
os.close(fd)
with open(tmp_path, "w") as fh:
yaml.dump(config, fh)
context.yaml_path = tmp_path
+2 -1
View File
@@ -33,7 +33,8 @@ def step_create_temp_dir(context):
# Give this invocation a unique database so repeated calls within the
# same scenario (e.g. Background + explicit Given) never collide.
db_path = tempfile.mktemp(suffix=".db", prefix="cleveragents_")
fd, db_path = tempfile.mkstemp(suffix=".db", prefix="cleveragents_")
os.close(fd)
os.environ["CLEVERAGENTS_DATABASE_URL"] = f"sqlite:///{db_path}"
context._scenario_db_paths = getattr(context, "_scenario_db_paths", [])
context._scenario_db_paths.append(db_path)
+3 -1
View File
@@ -97,7 +97,9 @@ def step_check_provider_configured(context):
env_file = Path(".env")
temp_env = None
if env_file.exists():
temp_env = Path(tempfile.mktemp(suffix=".env"))
fd, _tmp_env = tempfile.mkstemp(suffix=".env")
os.close(fd)
temp_env = Path(_tmp_env)
shutil.move(str(env_file), str(temp_env))
try:
+2 -1
View File
@@ -1038,7 +1038,8 @@ def step_persisted_decision_service(context: Context) -> None:
"""Create a DecisionService backed by a real SQLite database via UnitOfWork."""
from cleveragents.infrastructure.database.unit_of_work import UnitOfWork
db_path = tempfile.mktemp(suffix=".db", prefix="dsvc_persisted_")
fd, db_path = tempfile.mkstemp(suffix=".db", prefix="dsvc_persisted_")
os.close(fd)
context._dsvc_db_path = db_path
uow = UnitOfWork(f"sqlite:///{db_path}")
uow.init_database()
@@ -182,7 +182,9 @@ def step_age_service(context: Context) -> None:
@when("I check the age description for a {hours:d} hour old file")
def step_age_hours(context: Context, hours: int) -> None:
tmp = Path(tempfile.mktemp(prefix="ca-age-test-"))
fd, _tmp = tempfile.mkstemp(prefix="ca-age-test-")
os.close(fd)
tmp = Path(_tmp)
tmp.write_text("test")
old_time = time.time() - (hours * 3600)
os.utime(tmp, (old_time, old_time))
@@ -192,7 +194,9 @@ def step_age_hours(context: Context, hours: int) -> None:
@when("I check the age description for a {minutes:d} minute old file")
def step_age_minutes(context: Context, minutes: int) -> None:
tmp = Path(tempfile.mktemp(prefix="ca-age-test-"))
fd, _tmp = tempfile.mkstemp(prefix="ca-age-test-")
os.close(fd)
tmp = Path(_tmp)
tmp.write_text("test")
old_time = time.time() - (minutes * 60)
os.utime(tmp, (old_time, old_time))
@@ -202,7 +206,9 @@ def step_age_minutes(context: Context, minutes: int) -> None:
@when("I check the age description for a {days:d} day old file")
def step_age_days(context: Context, days: int) -> None:
tmp = Path(tempfile.mktemp(prefix="ca-age-test-"))
fd, _tmp = tempfile.mkstemp(prefix="ca-age-test-")
os.close(fd)
tmp = Path(_tmp)
tmp.write_text("test")
old_time = time.time() - (days * 86400)
os.utime(tmp, (old_time, old_time))
@@ -7,6 +7,7 @@ close-reopen cycle exercises real disk persistence.
from __future__ import annotations
import os
import tempfile
from datetime import UTC, datetime
from pathlib import Path
@@ -46,7 +47,8 @@ def _state_from_str(value: str) -> ProcessingState:
@given("a fresh file-backed persistence alignment database")
def step_fresh_db(context: Context) -> None:
tmp_path: str = tempfile.mktemp(suffix=".db")
fd, tmp_path = tempfile.mkstemp(suffix=".db")
os.close(fd)
db_url: str = f"sqlite:///{tmp_path}"
engine: Any = init_database(db_url)
session: Session = Session(bind=engine)
+3 -1
View File
@@ -6,6 +6,7 @@ list filters, plan tree links, and cross-restart durability.
from __future__ import annotations
import os
import tempfile
from datetime import UTC, datetime
from pathlib import Path
@@ -101,7 +102,8 @@ def _setup_db(context: Context, *, file_based: bool = False) -> None:
file.
"""
if file_based:
tmp = tempfile.mktemp(suffix=".db")
fd, tmp = tempfile.mkstemp(suffix=".db")
os.close(fd)
db_url = f"sqlite:///{tmp}"
context._pp_db_path = tmp
else:
@@ -9,6 +9,7 @@ resource_edges). Uses an in-memory SQLite database with schema created via
from __future__ import annotations
import json
import os
from datetime import UTC, datetime
from typing import Any
@@ -944,7 +945,8 @@ def step_run_init_database(context: Any) -> None:
from cleveragents.infrastructure.database.models import init_database
tmp = tempfile.mktemp(suffix=".db")
fd, tmp = tempfile.mkstemp(suffix=".db")
os.close(fd)
db_url = f"sqlite:///{tmp}"
engine = init_database(db_url)
context.rr_migrated_engine = engine
@@ -954,11 +956,13 @@ def step_run_init_database(context: Any) -> None:
@when("I run init_database twice on the same database file")
def step_run_init_database_twice(context: Any) -> None:
"""Run init_database twice on the same file to verify idempotency."""
import os
import tempfile
from cleveragents.infrastructure.database.models import init_database
tmp = tempfile.mktemp(suffix=".db")
fd, tmp = tempfile.mkstemp(suffix=".db")
os.close(fd)
db_url = f"sqlite:///{tmp}"
engine1 = init_database(db_url)
engine1.dispose()
+7 -2
View File
@@ -2,6 +2,7 @@
from __future__ import annotations
import os
import shutil
import tempfile
from pathlib import Path
@@ -437,7 +438,9 @@ def step_registration_returns(
@when("I scan a non-directory path for agent skills")
def step_scan_non_dir(context: Context) -> None:
tmpfile = Path(tempfile.mktemp(suffix=".txt"))
fd, _tmp = tempfile.mkstemp(suffix=".txt")
os.close(fd)
tmpfile = Path(_tmp)
tmpfile.write_text("not a directory", encoding="utf-8")
context.add_cleanup(lambda: tmpfile.unlink(missing_ok=True))
context.discovered = scan_directory(tmpfile)
@@ -445,7 +448,9 @@ def step_scan_non_dir(context: Context) -> None:
@when("I run discovery on a path that is a file")
def step_discover_file_path(context: Context) -> None:
tmpfile = Path(tempfile.mktemp(suffix=".txt"))
fd, _tmp = tempfile.mkstemp(suffix=".txt")
os.close(fd)
tmpfile = Path(_tmp)
tmpfile.write_text("not a directory", encoding="utf-8")
context.add_cleanup(lambda: tmpfile.unlink(missing_ok=True))
context.discovery_result = discover_agent_skills([tmpfile])
+3 -1
View File
@@ -2,6 +2,7 @@
from __future__ import annotations
import os
import sys
import tempfile
from pathlib import Path
@@ -87,7 +88,8 @@ def _test_from_yaml() -> None:
"name": "test/yaml",
"guards": {"max_tool_calls_per_step": 5},
}
tmp_path = tempfile.mktemp(suffix=".yaml")
fd, tmp_path = tempfile.mkstemp(suffix=".yaml")
os.close(fd)
with open(tmp_path, "w") as fh:
yaml.dump(cfg, fh)
p = AutomationProfile.from_yaml(tmp_path)
+3 -1
View File
@@ -8,6 +8,7 @@ Covers the integration between:
from __future__ import annotations
import os
import sys
import tempfile
from datetime import UTC, datetime
@@ -34,7 +35,8 @@ from cleveragents.infrastructure.database.models import (
def _create_temp_db() -> tuple[Session, str]:
"""Create a temporary SQLite database for testing."""
tmp = tempfile.mktemp(suffix=".db")
fd, tmp = tempfile.mkstemp(suffix=".db")
os.close(fd)
db_url = f"sqlite:///{tmp}"
engine = init_database(db_url)
session = Session(bind=engine)
+3 -1
View File
@@ -19,6 +19,7 @@ Subcommands:
from __future__ import annotations
import os
import sys
import tempfile
from datetime import UTC, datetime
@@ -60,7 +61,8 @@ from cleveragents.infrastructure.database.repositories import ( # noqa: E402
def _temp_db() -> tuple[str, Session]:
"""Create a file-backed temp SQLite DB and return (path, session)."""
tmp: str = tempfile.mktemp(suffix=".db")
fd, tmp = tempfile.mkstemp(suffix=".db")
os.close(fd)
db_url: str = f"sqlite:///{tmp}"
engine = init_database(db_url)
session = Session(bind=engine)
+3 -1
View File
@@ -10,6 +10,7 @@ Covers:
from __future__ import annotations
import os
import sys
import tempfile
from datetime import UTC, datetime
@@ -45,7 +46,8 @@ from cleveragents.infrastructure.database.repositories import (
def _create_temp_db() -> tuple[str, Session]:
"""Create a temporary SQLite database and return (path, session)."""
tmp = tempfile.mktemp(suffix=".db")
fd, tmp = tempfile.mkstemp(suffix=".db")
os.close(fd)
db_url = f"sqlite:///{tmp}"
engine = init_database(db_url)
session = Session(bind=engine)
+3 -1
View File
@@ -6,6 +6,7 @@ and Apply terminal processing states.
from __future__ import annotations
import os
import sys
import tempfile
from datetime import UTC, datetime
@@ -31,7 +32,8 @@ from cleveragents.infrastructure.database.models import (
def _create_temp_db() -> tuple[Session, str]:
"""Create a temporary SQLite database for testing."""
tmp = tempfile.mktemp(suffix=".db")
fd, tmp = tempfile.mkstemp(suffix=".db")
os.close(fd)
db_url = f"sqlite:///{tmp}"
engine = init_database(db_url)
session = Session(bind=engine)
+7 -3
View File
@@ -13,6 +13,7 @@ calling Robot test can assert on ``stdout``.
from __future__ import annotations
import os
import sys
import tempfile
from pathlib import Path
@@ -24,7 +25,8 @@ from cleveragents.infrastructure.database.models import init_database
def _schema_creates_tables() -> None:
"""Verify that init_database creates resource registry tables."""
tmp = tempfile.mktemp(suffix=".db")
fd, tmp = tempfile.mkstemp(suffix=".db")
os.close(fd)
db_url = f"sqlite:///{tmp}"
try:
engine = init_database(db_url)
@@ -41,7 +43,8 @@ def _schema_creates_tables() -> None:
def _table_has_expected_columns() -> None:
"""Verify core columns exist on each resource registry table."""
tmp = tempfile.mktemp(suffix=".db")
fd, tmp = tempfile.mkstemp(suffix=".db")
os.close(fd)
db_url = f"sqlite:///{tmp}"
try:
engine = init_database(db_url)
@@ -83,7 +86,8 @@ def _table_has_expected_columns() -> None:
def _migration_idempotent() -> None:
"""Verify that calling init_database twice is safe (idempotent)."""
tmp = tempfile.mktemp(suffix=".db")
fd, tmp = tempfile.mkstemp(suffix=".db")
os.close(fd)
db_url = f"sqlite:///{tmp}"
try:
engine1 = init_database(db_url)