Compare commits

...

1 Commits

Author SHA1 Message Date
CoreRasurae 532f128a49 refactor(test): replace deprecated tempfile.mktemp() with tempfile.mkstemp()
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 14s
CI / quality (pull_request) Successful in 17s
CI / build (pull_request) Successful in 25s
CI / typecheck (pull_request) Successful in 38s
CI / security (pull_request) Successful in 43s
CI / unit_tests (pull_request) Successful in 3m7s
CI / docker (pull_request) Successful in 41s
CI / integration_tests (pull_request) Successful in 5m3s
CI / coverage (pull_request) Successful in 5m26s
CI / benchmark-regression (pull_request) Successful in 38m33s
Replaced all 28 uses of the deprecated tempfile.mktemp() function with the
atomic tempfile.mkstemp() + os.close(fd) pattern across 16 test infrastructure
and benchmark files.  Added import os where it was not already present.

tempfile.mktemp() has been deprecated since Python 2.3 due to a TOCTOU race
condition: between name generation and file creation, another process could
create a file at the predicted path.  Under 16-worker parallel test execution,
there was also a non-zero risk of path collisions.

Files modified:
- features/environment.py (3 call sites)
- robot/helper_resource_registry_migration.py (3 call sites)
- robot/helper_plan_phase_migration.py (1 call site)
- robot/helper_plan_persistence_e2e.py (1 call site)
- robot/helper_persistence_lifecycle.py (1 call site)
- robot/helper_db_lifecycle_models.py (1 call site)
- robot/helper_automation_profiles.py (1 call site)
- features/steps/skill_discovery_steps.py (2 call sites)
- features/steps/resource_registry_tables_steps.py (2 call sites)
- features/steps/plan_persistence_steps.py (1 call site)
- features/steps/persistence_robot_alignment_steps.py (1 call site)
- features/steps/garbage_collection_cli_steps.py (3 call sites)
- features/steps/decision_recording_steps.py (1 call site)
- features/steps/coverage_boost_steps.py (1 call site)
- features/steps/cli_streaming_steps.py (1 call site)
- features/steps/automation_profiles_guards_steps.py (1 call site)
- benchmarks/persistence_robot_bench.py (4 call sites)

All 10,640 BDD scenarios pass.  No performance regression observed (within
normal run-to-run variance).  Zero remaining uses of tempfile.mktemp() in
the codebase.

ISSUES CLOSED: #730
2026-03-12 14:29:01 +00:00
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)