cleveragents/cleveragents-core
4
0
Fork 3
Code Issues 6.0k Pull Requests 115 Actions Packages Projects Releases Wiki Activity

fix(test): skip redundant Alembic migration check for existing test databases #727

Merged
CoreRasurae merged 1 commits from fix/test-db-performance-regression into master 2026-03-12 13:20:14 +00:00
Conversation 4 Commits 1 Files Changed 1
+6 -1
1 changed files with 6 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
features/environment.py
+6 -1
View File
7
 @@ -213,8 +213,13 @@ def _install_template_db_patch() -> None:
# Only copy template for databases that don't exist yet or are empty
# (SQLite auto-creates a 0-byte file on first engine open).
# If the DB already exists and is non-empty, it was either copied from
# the template or created by a prior step — either way it is already
# fully migrated. Skipping the Alembic check avoids redundant engine
# creation, SQLite lock contention, and the cumulative overhead that
# causes intermittent hangs in parallel test runs.
if db_path.exists() and db_path.stat().st_size > 0:
return _original_init_or_upgrade(self, **kwargs)
return
hurui200320 commented 2026-03-12 12:44:07 +00:00
Review
Copy Link
Copy Source

Bug Detection — Finding 1 (Low): Replacing _original_init_or_upgrade(self, **kwargs) with a bare return removes the safety net that would catch a stale template DB on subsequent init_or_upgrade calls within the same scenario.

When this matters: Only if build/.template-migrated.db was cached from a previous schema version (developer adds migrations but doesn't rebuild template). The original code would have detected pending migrations on the second call and applied them.

Why Low severity: The first call (which copies the template at line 226) never ran the original migration runner in either version — so this safety net was always incomplete. A stale template would cause immediate test failures, not silent data corruption.

Suggestion: Consider adding a freshness check in _ensure_template_db() (e.g. compare the template's Alembic stamp against ScriptDirectory.get_current_head()) to proactively detect stale templates, rather than relying on the migration runner as a fallback.

**Bug Detection — Finding 1 (Low):** Replacing `_original_init_or_upgrade(self, **kwargs)` with a bare `return` removes the safety net that would catch a stale template DB on subsequent `init_or_upgrade` calls within the same scenario. **When this matters:** Only if `build/.template-migrated.db` was cached from a previous schema version (developer adds migrations but doesn't rebuild template). The original code would have detected pending migrations on the second call and applied them. **Why Low severity:** The first call (which copies the template at line 226) never ran the original migration runner in either version — so this safety net was always incomplete. A stale template would cause immediate test failures, not silent data corruption. **Suggestion:** Consider adding a freshness check in `_ensure_template_db()` (e.g. compare the template's Alembic stamp against `ScriptDirectory.get_current_head()`) to proactively detect stale templates, rather than relying on the migration runner as a fallback.
# Copy the template — creates a fully-migrated DB in ~1ms
db_path.parent.mkdir(parents=True, exist_ok=True)